CVE-2022-21191: Other (NVD-CWE-Other)
value: Versions of the package global-modules-path before 3.0.0 are vulnerable to Command Injection due to missing input sanitization or other checks and sandboxes being employed to the getPath  function.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 9.8
Published: 2023-01-13T05:15Z
Repository: https://github.com/rosen-vladimirov/global-modules-path
Language: JavaScript
Files changed: lib/index.js, test/index.js
Commit date: 2023-01-08 19:05:12 +0200
Commit message: fix: do not allow command injection

Due to usage of execSync, callers may inject commands, like getPath("something & touch abc", "somethingElse & touch def").
To fix this, replace execSync with spawnSync, which is safe for command injection.

Diff: diff --git a/lib/index.js b/lib/index.js
index d6e5de5..ca1bc1d 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -23,13 +23,30 @@ const getNpmExecutable = (platform) => {
 	}
 
 	return npmExecutableName;
 };
 
+const spawnSyncWrapper = (command, commandArgs) => {
+	const result = childProcess.spawnSync(command, commandArgs);
+	if (!result) {
+		return null;
+	}
+
+	if (result.error) {
+		throw result.error;
+	}
+
+	if (result.stdout) {
+		return result.stdout.toString().trim();
+	}
+
+	return null;
+};
+
 const getNpmPrefix = (pathToNpm) => {
 	try {
-		const npmPrefixStdout = childProcess.execSync(`${pathToNpm} config get prefix`);
+		const npmPrefixStdout = spawnSyncWrapper(pathToNpm, ["config", "get", "prefix"]);
 		return npmPrefixStdout && npmPrefixStdout.toString().trim();
 	} catch (err) {
 		console.error(err.message);
 	}
 
@@ -47,10 +64,11 @@ const getPathFromNpmConfig = (platform, packageName) => {
 			nodeModulesPath = path.join(npmConfigPrefix, "lib", nodeModulesDirName);
 		}
 
 		const packagePath = path.join(nodeModulesPath, packageName);
 		const verifiedPath = getVerifiedPath(packagePath, packageName);
+
 		if (verifiedPath) {
 			return verifiedPath;
 		}
 	}
 
@@ -105,11 +123,12 @@ const getVerifiedPath = (suggestedPath, packageName) => {
 	}
 };
 
 const getPathFromExecutableNameOnWindows = (packageName, executableName) => {
 	try {
-		const whereResult = (childProcess.execSync(`where ${executableName}`) || "").toString().split("\n");
+		const whereResult = (spawnSyncWrapper("where", executableName) || "").split("\n");
+
 		for (const line of whereResult) {
 			const pathToExecutable = line && line.trim();
 
 			if (pathToExecutable) {
 				const pathToLib = path.join(path.dirname(pathToExecutable), nodeModulesDirName, packageName);
@@ -143,12 +162,12 @@ const getPathFromExecutableNameOnNonWindows = (packageName, executableName) => {
 		// Second way to find it is to use the result of which command
 		// It will give path to the executable, which is a symlink in fact, so we can get the full path from it:
 
 		// whichResult: /usr/local/nvm/versions/node/v4.2.1/bin/mobile-cli-lib
 		// lsLResult: lrwxrwxrwx 1 rvladimirov rvladimirov 52 Oct 20 14:51 /usr/local/nvm/versions/node/v4.2.1/bin/mobile-cli-lib -> ../lib/node_modules/mobile-cli-lib/bin/common-lib.js
-		const whichResult = (childProcess.execSync(`which ${executableName}`) || "").toString().trim(),
-			lsLResult = (childProcess.execSync(`ls -l \`which ${executableName}\``) || "").toString().trim();
+		const whichResult = spawnSyncWrapper("which", [executableName]);
+		const lsLResult = spawnSyncWrapper("ls", ["-l", whichResult]);
 
 		if (whichResult && lsLResult) {
 			const regex = new RegExp(`${whichResult}\\s+->\\s+(.*?)$`),
 				match = lsLResult.match(regex);
 
diff --git a/test/index.js b/test/index.js
index b1b34c9..2988bfb 100644
--- a/test/index.js
+++ b/test/index.js
@@ -10,20 +10,20 @@ const index = require("../lib/index");
 
 let processWrapper = require("../lib/process-wrapper");
 let fs = require("fs"),
 	childProcess = require("child_process");
 
-const originalExecSync = childProcess.execSync;
+const originalspawnSync = childProcess.spawnSync;
 const originalConsoleError = console.error;
 const originalExistsSync = fs.existsSync;
 const originalReadFileSync = fs.readFileSync;
 
 describe("getPath", () => {
 
 	afterEach(() => {
 		processWrapper.getProcessPlatform = () => process.platform;
-		childProcess.execSync = originalExecSync;
+		childProcess.spawnSync = originalspawnSync;
 		console.error = originalConsoleError;
 		fs.existsSync = originalExistsSync;
 		fs.readFileSync = originalReadFileSync;
 	});
 
@@ -39,13 +39,13 @@ describe("getPath", () => {
 			describe("works correctly when npm prefix is used", () => {
 				it("does not throw error when getting npm prefix throws", () => {
 					const expectedErrorMessage = "npm throws";
 					let errors = [];
 					console.error = (message) => errors.push(message);
-					childProcess.execSync = (command) => {
-						if (command.indexOf("get prefix") !== -1) {
-							throw new Error(expectedErrorMessage);
+					childProcess.spawnSync = (command, commandArgs) => {
+						if (commandArgs.indexOf("get") !== -1 && commandArgs.indexOf("prefix") !== -1) {
+							return { error: new Error(expectedErrorMessage) };
 						}
 					};
 
 					// Do not pass executable name here as we want to test only getting npm prefix.
 					index.getPath("test1");
@@ -57,11 +57,11 @@ describe("getPath", () => {
 					const npmConfigPrefix = "/npm/dir",
 						packageName = "test1";
 
 					fs.existsSync = () => false;
 
-					childProcess.execSync = (command) => {
+					childProcess.spawnSync = (command) => {
 						if (command.indexOf("get prefix") !== -1) {
 							return npmConfigPrefix;
 						}
 					};
 
@@ -73,11 +73,11 @@ describe("getPath", () => {
 			describe("works correctly when executableName is used", () => {
 				it("returns null when where result fails", () => {
 					const packageName = "test1",
 						executableName = "test1.js";
 
-					childProcess.execSync = () => null;
+					childProcess.spawnSync = () => null;
 
 					const result = index.getPath(packageName, executableName);
 					assert.deepEqual(result, null);
 				});
 			});
@@ -85,13 +85,14 @@ describe("getPath", () => {
 	});
 
 	const getCorrectResultFromNpmPrefix = (npmConfigPrefix, packageName) => {
 		fs.existsSync = () => true;
 		fs.readFileSync = () => JSON.stringify({ name: packageName });
-		childProcess.execSync = (command) => {
-			if (command.indexOf("get prefix") !== -1) {
-				return npmConfigPrefix;
+		fs.realpathSync = (p) => p; 
+		childProcess.spawnSync = (command, commandArgs) => {
+			if (commandArgs.indexOf("get") !== -1 && commandArgs.indexOf("prefix") !== -1) {
+				return { stdout: npmConfigPrefix };
 			}
 		};
 
 		return index.getPath(packageName);
 	};
@@ -103,12 +104,12 @@ describe("getPath", () => {
 		});
 
 		describe("works correctly when npm prefix is used", () => {
 			it("uses npm.cmd for npm executable", () => {
 				let commands = [];
-				childProcess.execSync = (command) => {
-					commands.push(command);
+				childProcess.spawnSync = (command, commandArgs) => {
+					commands.push(`${command} ${(commandArgs || []).join(" ")}`);
 				};
 
 				index.getPath("test1");
 				assert.isTrue(commands.indexOf("npm.cmd config get prefix") !== -1);
 			});
@@ -139,13 +140,13 @@ describe("getPath", () => {
 				let errors = [];
 				console.error = (message) => errors.push(message);
 
 				fs.existsSync = () => true;
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						throw new Error(expectedErrorMessage);
+						return { error: new Error(expectedErrorMessage) };
 					}
 
 					return null;
 				};
 
@@ -161,13 +162,13 @@ describe("getPath", () => {
 					executableDirName = path.join("C:", "Users", "username", "AppData", "Roaming", "npm"),
 					whereResult = path.join(executableDirName, executableName);
 
 				fs.existsSync = () => true;
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -192,13 +193,13 @@ describe("getPath", () => {
 					executableDirName = path.join(yarnDirPath, "bin"),
 					whereResult = path.join(executableDirName, executableName);
 
 				fs.existsSync = (pathToCheck) => pathToCheck !== path.join(executableDirName, "node_modules", packageName);
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -231,13 +232,13 @@ describe("getPath", () => {
 					executableDirName = path.join(nodeModulesDirName, ".bin"),
 					whereResult = path.join(executableDirName, executableName);
 
 				fs.existsSync = (pathToCheck) => pathToCheck !== path.join(executableDirName, "node_modules", packageName);
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -282,13 +283,13 @@ describe("getPath", () => {
 					}
 
 					return "";
 				};
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -314,13 +315,13 @@ describe("getPath", () => {
 					}
 
 					return "";
 				};
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -352,13 +353,13 @@ describe("getPath", () => {
 					}
 
 					return "";
 				};
 
-				childProcess.execSync = (command) => {
+				childProcess.spawnSync = (command) => {
 					if (command.indexOf("where") !== -1) {
-						return whereResult;
+						return { stdout: whereResult };
 					}
 
 					return null;
 				};
 
@@ -378,12 +379,12 @@ describe("getPath", () => {
 				processWrapper.getProcessPlatform = () => platform;
 			});
 
 			it("uses npm for npm executable", () => {
 				let commands = [];
-				childProcess.execSync = (command) => {
-					commands.push(command);
+				childProcess.spawnSync = (command, commandArgs) => {
+					commands.push(`${command} ${(commandArgs || []).join(" ")}`);
 				};
 
 				index.getPath("test1");
 
 				assert.isTrue(commands.indexOf("npm config get prefix") !== -1);
@@ -414,11 +415,11 @@ describe("getPath", () => {
 					let errors = [];
 					console.error = (message) => errors.push(message);
 
 					fs.existsSync = () => true;
 
-					childProcess.execSync = (command) => {
+					childProcess.spawnSync = (command) => {
 						if (command.indexOf("which") !== -1) {
 							throw new Error(expectedErrorMessage);
 						}
 
 						return null;
@@ -438,13 +439,13 @@ describe("getPath", () => {
 					let errors = [];
 					console.error = (message) => errors.push(message);
 
 					fs.existsSync = () => true;
 
-					childProcess.execSync = (command) => {
-						if (command.indexOf("ls -l") !== -1) {
-							throw new Error(expectedErrorMessage);
+					childProcess.spawnSync = (command) => {
+						if (command === "ls") {
+							return { error: new Error(expectedErrorMessage) };
 						}
 
 						return null;
 					};
 
@@ -477,18 +478,18 @@ describe("getPath", () => {
 
 						fs.realpathSync = (filePath) => {
 							return filePath;
 						};
 
-						childProcess.execSync = (command) => {
+						childProcess.spawnSync = (command) => {
 
-							if (command.indexOf("ls -l") !== -1) {
-								return lsLResult;
+							if (command === "ls") {
+								return { stdout: lsLResult };
 							}
 
-							if (command.indexOf("which") !== -1) {
-								return whichResult;
+							if (command == "which") {
+								return { stdout: whichResult };
 							}
 
 							return null;
 						};
 					};
@@ -542,18 +543,18 @@ describe("getPath", () => {
 
 						fs.existsSync = (filePath) => {
 							return filePath.indexOf("package.json") !== -1;
 						};
 
-						childProcess.execSync = (command) => {
+						childProcess.spawnSync = (command) => {
 
-							if (command.indexOf("ls -l") !== -1) {
-								return lsLResult;
+							if (command === "ls") {
+								return { stdout: lsLResult };
 							}
 
-							if (command.indexOf("which") !== -1) {
-								return whichResult;
+							if (command == "which") {
+								return { stdout: whichResult };
 							}
 
 							return null;
 						};
 
@@ -580,18 +581,18 @@ describe("getPath", () => {
 
 						fs.existsSync = (filePath) => {
 							return filePath.indexOf("package.json") !== -1;
 						};
 
-						childProcess.execSync = (command) => {
+						childProcess.spawnSync = (command) => {
 
-							if (command.indexOf("ls -l") !== -1) {
-								return lsLResult;
+							if (command === "ls") {
+								return { stdout: lsLResult };
 							}
 
-							if (command.indexOf("which") !== -1) {
-								return whichResult;
+							if (command == "which") {
+								return { stdout: whichResult };
 							}
 
 							return null;
 						};
 

Vulnerable code: 		const npmPrefixStdout = childProcess.execSync(`${pathToNpm} config get prefix`);
		const whereResult = (childProcess.execSync(`where ${executableName}`) || "").toString().split("\n");
		const whichResult = (childProcess.execSync(`which ${executableName}`) || "").toString().trim(),
			lsLResult = (childProcess.execSync(`ls -l \`which ${executableName}\``) || "").toString().trim();
const originalExecSync = childProcess.execSync;
		childProcess.execSync = originalExecSync;
					childProcess.execSync = (command) => {
						if (command.indexOf("get prefix") !== -1) {
							throw new Error(expectedErrorMessage);
					childProcess.execSync = (command) => {
					childProcess.execSync = () => null;
		childProcess.execSync = (command) => {
			if (command.indexOf("get prefix") !== -1) {
				return npmConfigPrefix;
				childProcess.execSync = (command) => {
					commands.push(command);
				childProcess.execSync = (command) => {
						throw new Error(expectedErrorMessage);
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
						return whereResult;
				childProcess.execSync = (command) => {
					commands.push(command);
					childProcess.execSync = (command) => {
					childProcess.execSync = (command) => {
						if (command.indexOf("ls -l") !== -1) {
							throw new Error(expectedErrorMessage);
						childProcess.execSync = (command) => {
							if (command.indexOf("ls -l") !== -1) {
								return lsLResult;
							if (command.indexOf("which") !== -1) {
								return whichResult;
						childProcess.execSync = (command) => {
							if (command.indexOf("ls -l") !== -1) {
								return lsLResult;
							if (command.indexOf("which") !== -1) {
								return whichResult;
						childProcess.execSync = (command) => {
							if (command.indexOf("ls -l") !== -1) {
								return lsLResult;
							if (command.indexOf("which") !== -1) {
								return whichResult;
Fixed code: const spawnSyncWrapper = (command, commandArgs) => {
	const result = childProcess.spawnSync(command, commandArgs);
	if (!result) {
		return null;
	}

	if (result.error) {
		throw result.error;
	}

	if (result.stdout) {
		return result.stdout.toString().trim();
	}

	return null;
};

		const npmPrefixStdout = spawnSyncWrapper(pathToNpm, ["config", "get", "prefix"]);

		const whereResult = (spawnSyncWrapper("where", executableName) || "").split("\n");

		const whichResult = spawnSyncWrapper("which", [executableName]);
		const lsLResult = spawnSyncWrapper("ls", ["-l", whichResult]);
const originalspawnSync = childProcess.spawnSync;
		childProcess.spawnSync = originalspawnSync;
					childProcess.spawnSync = (command, commandArgs) => {
						if (commandArgs.indexOf("get") !== -1 && commandArgs.indexOf("prefix") !== -1) {
							return { error: new Error(expectedErrorMessage) };
					childProcess.spawnSync = (command) => {
					childProcess.spawnSync = () => null;
		fs.realpathSync = (p) => p; 
		childProcess.spawnSync = (command, commandArgs) => {
			if (commandArgs.indexOf("get") !== -1 && commandArgs.indexOf("prefix") !== -1) {
				return { stdout: npmConfigPrefix };
				childProcess.spawnSync = (command, commandArgs) => {
					commands.push(`${command} ${(commandArgs || []).join(" ")}`);
				childProcess.spawnSync = (command) => {
						return { error: new Error(expectedErrorMessage) };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command) => {
						return { stdout: whereResult };
				childProcess.spawnSync = (command, commandArgs) => {
					commands.push(`${command} ${(commandArgs || []).join(" ")}`);
					childProcess.spawnSync = (command) => {
					childProcess.spawnSync = (command) => {
						if (command === "ls") {
							return { error: new Error(expectedErrorMessage) };
						childProcess.spawnSync = (command) => {
							if (command === "ls") {
								return { stdout: lsLResult };
							if (command == "which") {
								return { stdout: whichResult };
						childProcess.spawnSync = (command) => {
							if (command === "ls") {
								return { stdout: lsLResult };
							if (command == "which") {
								return { stdout: whichResult };
						childProcess.spawnSync = (command) => {
							if (command === "ls") {
								return { stdout: lsLResult };
							if (command == "which") {
								return { stdout: whichResult };