• VSCode Version: 1.49.2
• OS Version: Can be reproduced on Debian Linux and Windows

https://portal.msrc.microsoft.com/en-US/security-guidance/advisory/CVE-2020-16881. says that "A remote code execution vulnerability exists in Visual Studio Code when a user is tricked into opening a malicious 'package.json' file."

The advisory links to https://code.visualstudio.com/updates/v1_48. There is no mention of "CVE" or "Security" on this page, but it links to https://github.com/microsoft/vscode/issues?q=is%3Aissue+milestone%3A%22July+2020+Recovery+2%22+is%3Aclosed+ which links to #105334 which is a code change in the realm of handling package.json files.

The vulnerability

The Microsoft advisory, VSCode's release notes, and VSCode's issues/commits do not make it obvious what the vulnerability was, or what the fix for it was, so I am making the following assumptions.

If my assumptions are wrong, please let me know. In particular, if the root cause for CVE-2020-16881 is something else, what is it?

I assume that the fix for CVE-2020-16881 is the commit that fixes #105334

I assume that the vulnerability was as follows:

1. Given a package.json file where a package name contains shell metacharacters; and
2. When a user hovers over that package name, VSCode does:

child_process.exec('npm view --json ' + pack + ' description dist-tags.latest homepage version', [... SNIP ...]

where pack is the name of the package

child_process.exec() is a system()-like function, which executes the string argument using the system's shell. It is hence vulnerable to command injection. See https://cwe.mitre.org/data/definitions/78.html and https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

For example, the following Proof of Concept package.json file when viewed as part of a workspace in VSCode <1.48.2 will launch the Gnome Calculator on Linux when the user hovers over the package name:

{
    "dependencies":{
        "hover_over_me_$(gnome-calculator)": "bar"
    }
}

The original fix

88664e2 adds a regex filter as follows:

private isValidNPMName(name: string): boolean {
		// following rules from https://github.com/npm/validate-npm-package-name
		if (!name || name.length > 214 || name.match(/^[_.]/)) {
			return false;
		}
		const match = name.match(/^(?:@([^/]+?)[/])?([^/]+?)$/);
		if (match) {
			const scope = match[1];
			if (scope && encodeURIComponent(scope) !== scope) {
				return false;
			}
			const name = match[2];
			return encodeURIComponent(name) === name;
		}
		return true;
	}

This filter appears to be intended to prevent the lookup of invalid package names, which breaks Proof of Concept files such as the above. It does so by extracting components of the package name using capture groups, and then checking to see that they only contain characters that will not be transformed by encodeURIComponent() (i.e. checks to ensure that they contain only URL-safe characters)

The bypass

The regex filtering function has a "fail open" behaviour. If the capture groups fail to extract the expected components of the package name, the function will return true.

This behaviour can be triggered by adding a leading slash to the malicious package name.

For example, the following Proof of Concept package.json file when viewed as part of a workspace in the current version of VSCode will launch the Gnome Calculator on Linux when the user hovers over the package name:

{
    "dependencies":{
        "/hover_over_me_$(gnome-calculator)": "bar"
    }
}

Recommended fix

child_process.exec() is a system()-like function, in that it executes command as a string using the system's shell. It is safer to use child_process.execFile() which is an exec*()-style function, which accepts an array for the child process' argv rather than using a shell to launch the process. By avoiding the use of the shell, the shell injection vulnerability is resolved regardless of the package name being looked up.

I will submit a PR that implements this fix.

Does this issue occur when all extensions are disabled?: Yes