Limit allowed packageManager values

mgol · Nov 14, 2023 · 8fdc6ad · 8fdc6ad
1 parent 9809f13
commit 8fdc6ad
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 22 deletions.
diff --git a/README.md b/README.md
@@ -86,6 +86,8 @@ The `config` object may have the following fields:
 
 Package manager to check against. Possible values: `'npm'`, `'bower'`. (Note: for `bower` you need to have the `bower` package installed either globally or locally in the same project in which you use `check-dependencies`).
 
+**NOTE: The value passed to this parameter will be invoked if the `install` option is set to `true`. Do not pass untrusted input here. In the worst case, it may lead to arbitrary code execution! Also, versions below `1.1.1` did no validation of this parameter; versions `1.1.1` and newer ensure it matches the regex `/^[a-z][a-z0-9-]*$/i`. It is still not safe to provide untrusted input in versions `1.1.1` or newer, though.**
+
 Type: `string`
 
 Default: `'npm'`

diff --git a/lib/check-dependencies.js b/lib/check-dependencies.js
@@ -54,27 +54,6 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => {
     let installNeeded = false;
     let pruneNeeded = false;
 
-    const options = {
-        packageManager: 'npm',
-        onlySpecified: false,
-        install: false,
-        scopeList: ['dependencies', 'devDependencies'],
-        optionalScopeList: ['optionalDependencies'],
-        verbose: false,
-        checkGitUrls: false,
-        checkCustomPackageNames: false,
-        log: console.log.bind(console),
-        error: console.error.bind(console),
-        ...config,
-    };
-
-    const packageJsonName =
-        options.packageManager === 'npm' ? 'package.json' : 'bower.json';
-    const packageJsonRegex =
-        options.packageManager === 'npm' ? /package\.json$/ : /bower\.json$/;
-    depsDirName =
-        options.packageManager === 'npm' ? 'node_modules' : 'bower_components';
-
     const log = message => {
         output.log.push(message);
         if (options.verbose) {
@@ -104,6 +83,36 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => {
         return finish();
     };
 
+    const options = {
+        packageManager: 'npm',
+        onlySpecified: false,
+        install: false,
+        scopeList: ['dependencies', 'devDependencies'],
+        optionalScopeList: ['optionalDependencies'],
+        verbose: false,
+        checkGitUrls: false,
+        checkCustomPackageNames: false,
+        log: console.log.bind(console),
+        error: console.error.bind(console),
+        ...config,
+    };
+
+    if (!/^[a-z][a-z0-9-]*$/i.test(options.packageManager)) {
+        success = false;
+        error(
+            'The packageManager field value must match the regex ' +
+                `\`/^[a-z][a-z0-9-]*$/i\`; got: "${options.packageManager}"`,
+        );
+        return finish();
+    }
+
+    const packageJsonName =
+        options.packageManager === 'npm' ? 'package.json' : 'bower.json';
+    const packageJsonRegex =
+        options.packageManager === 'npm' ? /package\.json$/ : /bower\.json$/;
+    depsDirName =
+        options.packageManager === 'npm' ? 'node_modules' : 'bower_components';
+
     options.packageDir = options.packageDir || findup(packageJsonName);
     if (!options.packageDir) {
         return missingPackageJson();

diff --git a/test/spec.js b/test/spec.js
@@ -31,7 +31,11 @@ describe('checkDependencies', () => {
                         config = {};
                         args.unshift(config);
                     }
-                    config.packageManager = 'bower';
+
+                    // Allow specs to specify custom `packageManager` in tests.
+                    if (!config.packageManager) {
+                        config.packageManager = 'bower';
+                    }
                 }
 
                 if (checkDependenciesMode === 'callbacks') {
@@ -91,6 +95,24 @@ describe('checkDependencies', () => {
             'c: installed: 1.2.3, expected: <2.0',
         ];
 
+        it('should exit with an error for invalid `packageManager`', done => {
+            checkDeps(
+                {
+                    packageManager: 'foo bar',
+                    packageDir: `${fixturePrefixSeparate}ok`,
+                    scopeList: ['dependencies', 'devDependencies'],
+                },
+                output => {
+                    assert.deepEqual(output.error, [
+                        'The packageManager field value must match the regex ' +
+                            '`/^[a-z][a-z0-9-]*$/i`; got: "foo bar"',
+                    ]);
+                    assert.strictEqual(output.status, 1);
+                    done();
+                },
+            );
+        });
+
         it('should not print errors for valid package setup', done => {
             checkDeps(
                 {