CVE-2020-7735: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: The package ng-packagr before 10.1.1 are vulnerable to Command Injection via the styleIncludePaths option.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 6.6
Published: 2020-09-25T12:15Z
Repository: https://github.com/ng-packagr/ng-packagr
Language: TypeScript
Files changed: integration/samples/scss-paths/baz/baz component.less	, integration/samples/scss-paths/specs/metadata.ts, src/lib/styles/stylesheet-processor.ts
Commit date: 2020-09-25 13:51:21 +0200
Commit message: fix: replace execFile with execFileSync to fix a potential malicious cmd injection

Diff: diff --git a/integration/samples/scss-paths/baz/baz component.less b/integration/samples/scss-paths/baz/baz component.less
index 76a6379..c489db7 100644
--- a/integration/samples/scss-paths/baz/baz component.less	
+++ b/integration/samples/scss-paths/baz/baz component.less	
@@ -1,7 +1,6 @@
 @import 'theme';
-@import 'less/test/less/debug/linenumbers';
 
 .baz {
   .oom {
     color: @foreground-color-bright;
   }
diff --git a/integration/samples/scss-paths/specs/metadata.ts b/integration/samples/scss-paths/specs/metadata.ts
index 4de33ce..ca4f94c 100644
--- a/integration/samples/scss-paths/specs/metadata.ts
+++ b/integration/samples/scss-paths/specs/metadata.ts
@@ -31,11 +31,11 @@ describe(`@sample/scss-paths`, () => {
       const lessStyles = METADATA['metadata']['BazComponent']['decorators'][0]['arguments'][0]['styles'][1];
       expect(lessStyles).to.contain(`.baz .oom`);
       expect(lessStyles).to.contain(`color:red`);
     });
 
-    it(`should resolve the styles from the Less 'node_module' file ~`, () => {
+    xit(`should resolve the styles from the Less 'node_module' file ~`, () => {
       const lessStyles = METADATA['metadata']['BazComponent']['decorators'][0]['arguments'][0]['styles'][1];
       expect(lessStyles).to.contain(`tst3`);
     });
 
     it(`should resolve the styles from the Stylus theme`, () => {
diff --git a/src/lib/styles/stylesheet-processor.ts b/src/lib/styles/stylesheet-processor.ts
index 3d9a0a8..eb9b69c 100644
--- a/src/lib/styles/stylesheet-processor.ts
+++ b/src/lib/styles/stylesheet-processor.ts
@@ -1,8 +1,8 @@
 import * as path from 'path';
 import * as log from '../utils/log';
-import { execSync } from 'child_process';
+import { execFileSync } from 'child_process';
 
 // CSS Tools
 import * as autoprefixer from 'autoprefixer';
 import * as browserslist from 'browserslist';
 import * as nodeSassTildeImporter from 'node-sass-tilde-importer';
@@ -48,11 +48,11 @@ export class StylesheetProcessor {
       from: filePath,
       to: filePath.replace(path.extname(filePath), '.css'),
     });
 
     // Log warnings from postcss
-    result.warnings().forEach((msg) => log.warn(msg.toString()));
+    result.warnings().forEach(msg => log.warn(msg.toString()));
 
     return result.css;
   }
 
   private renderPreProcessor(filePath: string, content: string): string {
@@ -73,16 +73,16 @@ export class StylesheetProcessor {
           })
           .css.toString();
 
       case '.less':
         // this is the only way I found to make LESS sync
-        let cmd = `node "${require.resolve('less/bin/lessc')}" "${filePath}" --js`;
+        const args = [filePath, '--js'];
         if (this.styleIncludePaths.length) {
-          cmd += ` --include-path="${this.styleIncludePaths.join(':')}"`;
+          args.push(`--include-path=${this.styleIncludePaths.join(':')}`);
         }
 
-        return execSync(cmd).toString();
+        return execFileSync(require.resolve('less/bin/lessc'), args).toString();
 
       case '.styl':
       case '.stylus':
         return (
           stylus(content)
@@ -124,11 +124,11 @@ export class StylesheetProcessor {
 
     const asyncPlugins = ['postcss-svgo'];
     const cssNanoPlugins = preset.plugins
       // replicate the `initializePlugin` behavior from https://github.com/cssnano/cssnano/blob/a566cc5/packages/cssnano/src/index.js#L8
       .map(([creator, pluginConfig]) => creator(pluginConfig))
-      .filter((plugin) => !asyncPlugins.includes(plugin.postcssPlugin));
+      .filter(plugin => !asyncPlugins.includes(plugin.postcssPlugin));
 
     postCssPlugins.push(...cssNanoPlugins);
 
     return postcss(postCssPlugins);
   }

Vulnerable code: @import 'less/test/less/debug/linenumbers';
    it(`should resolve the styles from the Less 'node_module' file ~`, () => {
import { execSync } from 'child_process';
    result.warnings().forEach((msg) => log.warn(msg.toString()));
        let cmd = `node "${require.resolve('less/bin/lessc')}" "${filePath}" --js`;
          cmd += ` --include-path="${this.styleIncludePaths.join(':')}"`;
        return execSync(cmd).toString();
      .filter((plugin) => !asyncPlugins.includes(plugin.postcssPlugin));
Fixed code:     xit(`should resolve the styles from the Less 'node_module' file ~`, () => {
import { execFileSync } from 'child_process';
    result.warnings().forEach(msg => log.warn(msg.toString()));
        const args = [filePath, '--js'];
          args.push(`--include-path=${this.styleIncludePaths.join(':')}`);
        return execFileSync(require.resolve('less/bin/lessc'), args).toString();
      .filter(plugin => !asyncPlugins.includes(plugin.postcssPlugin));