Conversation
The transformMarkdown function fails to recognize standalone Carriage Return (\r) characters as line terminators. When processing Markdown, it incorrectly wraps text containing \r into a single JavaScript comment. Node.js, however, treats \r as a valid line break, causing any code following the \r to be executed instead of remaining commented out. This allows arbitrary code execution hidden within plain text sections of Markdown files. The vulnerability exists because of a mismatch between the parser's logic and the JavaScript engine's (Node.js) specification.
|
Since this involves unintended code execution via a parsing bypass, would you be open to opening a GitHub Security Advisory for this? |
| let codeBlockEnd = '' | ||
| let prevLineIsEmpty = true | ||
| for (const line of bufToString(buf).split(/\r?\n/)) { | ||
| for (const line of bufToString(buf).split(/\r?\n|\r|\u2028|\u2029/)) { |
There was a problem hiding this comment.
Let's simplify a bit: /\r\n|[\n\r\u2028\u2029]/
|
cool @antongolub and it's possible to have a GitHub Security Advisory for this? |
|
RCE is a feature, code verification before running is a mandatory requirement. We'll add a clear warning for this to the docs. |
|
yes but even a good code review leads to RCE, even if the code is commented out, execution is still possible. |
|
i update my fix, thanks @antongolub |
|
Please i program yo disclose my report from google vrp to my blog tomorrow can you merge a fix ? @antongolub @moQuez @precision @azu ? |
|
Follow the template, plz. https://github.com/google/zx/blob/main/.github/PULL_REQUEST_TEMPLATE.md |
}
▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]
src/cli.ts:263:20:
263 │ metaurl: string = import.meta.url,
╵ ~~~~~~~~~~~
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
added 546 packages, and audited 547 packages in 25s
132 packages are looking for funding
run `npm fund` for details
1 moderate severity vulnerability
To address all issues, run:
npm audit fix
Run `npm audit` for details.
It normal ? I can push if all https://github.com/google/zx/blob/main/.github/PULL_REQUEST_TEMPLATE.md are ok ? |
|
Fixes #issue / suggests an improvement for (const line of bufToString(buf).split(/\r?\n|\r|\u2028|\u2029/)) {Note : that a
|
|
cc @antongolub |
|
Hi, it's a disclose day, i wait you. @antongolub |
3 participants
Fixes ##1403 / suggests an improvement
This was discovered during a security audit of zx. Although Google VRP determined it doesn't meet their internal severity threshold for a 'security bug' tracking, it remains a functional flaw that allows unintended code execution. This PR resolves the parsing discrepancy.