-
Notifications
You must be signed in to change notification settings - Fork 218
Fix nodejs breaking change CVE-2024-27980 #973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
7606993
af842a2
3e93e3f
434754f
97f8b80
1188664
9e06c04
b1bde5a
f6eb247
dd558a9
c1ced1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -395,25 +395,59 @@ export async function versionBump(options: IVersionBumpOptions): Promise<void> { | |
} | ||
} | ||
|
||
|
||
// call `npm version` to do our dirty work | ||
const args = ['version', options.version]; | ||
|
||
if (options.commitMessage) { | ||
args.push('-m', options.commitMessage); | ||
const isWindows = process.platform === 'win32'; | ||
|
||
const commitMessage = isWindows ? sanitizeCommitMessage(options.commitMessage) : options.commitMessage; | ||
if (commitMessage) { | ||
args.push('-m', commitMessage); | ||
} | ||
|
||
if (!(options.gitTagVersion ?? true)) { | ||
args.push('--no-git-tag-version'); | ||
} | ||
|
||
const { stdout, stderr } = await promisify(cp.execFile)(process.platform === 'win32' ? 'npm.cmd' : 'npm', args, { cwd }); | ||
|
||
const { stdout, stderr } = await promisify(cp.execFile)(isWindows ? 'npm.cmd' : 'npm', args, { cwd, shell: isWindows /* https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2 */ }); | ||
if (!process.env['VSCE_TESTS']) { | ||
process.stdout.write(stdout); | ||
process.stderr.write(stderr); | ||
} | ||
} | ||
|
||
function sanitizeCommitMessage(message?: string): string | undefined { | ||
if (!message) { | ||
return undefined; | ||
} | ||
|
||
// Check for characters that might escape quotes or introduce shell commands. | ||
// Don't allow: ', ", `, $, \ (except for \n) | ||
const unsafeRegex = /(?<!\\)\\(?!n)|['"`$]/g; | ||
joaomoreno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Remove any unsafe characters found by the unsafeRegex | ||
const sanitizedMessage = message.replace(unsafeRegex, ''); | ||
|
||
// Additional check to make sure nothing potentially dangerous is still in the string | ||
if ([`'`, `"`, '`', '$'].some(char => sanitizedMessage.includes(char))) { | ||
benibenj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new Error('Commit message contains potentially dangerous characters after initial sanitization.'); | ||
} | ||
|
||
for (let index = 0; index < sanitizedMessage.length; index++) { | ||
const char = sanitizedMessage[index]; | ||
if (char === '\\' && sanitizedMessage[index + 1] !== 'n') { | ||
throw new Error('Commit message contains potentially dangerous characters after initial sanitization.'); | ||
} | ||
} | ||
|
||
if (sanitizedMessage.length === 0) { | ||
return undefined; | ||
} | ||
|
||
return `"${sanitizedMessage}"`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure that explicit quotes are really necessary when using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With |
||
} | ||
|
||
export const Targets = new Set([ | ||
'win32-x64', | ||
'win32-arm64', | ||
|
Uh oh!
There was an error while loading. Please reload this page.