CVE-2020-15123: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: In codecov (npm package) before version 3.7.1 the upload method has a command injection vulnerability. Clients of the codecov-node library are unlikely to be aware of this, so they might unwittingly write code that contains a vulnerability. A similar CVE (CVE-2020-7597 for GHSA-5q88-cjfq-g2mh) was issued but the fix was incomplete. It only blocked &, and command injection is still possible using backticks instead to bypass the sanitizer. The attack surface is low in this case. Particularly in the standard use of codecov, where the module is used directly in a build pipeline, not built against as a library in another application that may supply malicious input and perform command injection.
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: 9.3
Published: 2020-07-20T18:15Z
Repository: https://github.com/codecov/codecov-node
Language: Other
Files changed: .idea/.gitignore, README.md, lib/codecov.js
Commit date: 2020-07-17 12:20:03 -0400
Commit message: Switch from execSync to execFileSync (#180)

* Switch from execSync to execFileSync

* Call existsSync instead of executing `if`

* Trigger a build

* I think this fixes linux and macos...

* Only add more_patterns of it has a value. Correct windows.

* Remove default value to more_patterns, change windows to winPatterns

* Try a colon.

* Let's try not using MinGw...

* What are the chances that Windows has a find?

* Put it back

* Reverting Windows side so this can me merged in time if needed.

* Reverting Windows side so this can me merged in time if needed.

* Update lib/codecov.js

Co-authored-by: Guillaume St-Pierre <guillaume@codecov.io>

* Update lib/codecov.js

Co-authored-by: Guillaume St-Pierre <guillaume@codecov.io>

* Set initial values

Co-authored-by: Guillaume St-Pierre <guillaume@codecov.io>
Diff: diff --git a/.idea/.gitignore b/.idea/.gitignore
new file mode 100644
index 0000000..03edeb0
--- /dev/null
+++ b/.idea/.gitignore
@@ -0,0 +1,7 @@
+# Default ignored files
+/shelf/
+/workspace.xml
+# Editor-based HTTP Client requests
+/httpRequests/
+
+.idea/
\ No newline at end of file
diff --git a/README.md b/README.md
index a10cce0..62a5216 100644
--- a/README.md
+++ b/README.md
@@ -114,5 +114,7 @@ If you're seeing an **HTTP 400 error when uploading reports to S3**, make sure y
 - v3.6.1 Fix for Semaphore
 - v3.6.2 Command line args sanitized fix
 - v3.6.3 Fix for AWS Codebuild & package updates
 - v3.6.4 Fix Cirrus CI
 - v3.7.0 Remove the X-Amz-Acl: public-read header
+
+.
diff --git a/lib/codecov.js b/lib/codecov.js
index 2a8cc44..da8b6e5 100644
--- a/lib/codecov.js
+++ b/lib/codecov.js
@@ -2,36 +2,38 @@ var fs = require('fs')
 var path = require('path')
 var request = require('teeny-request').teenyRequest
 var urlgrey = require('urlgrey')
 var jsYaml = require('js-yaml')
 var walk = require('ignore-walk')
+var execFileSync = require('child_process').execFileSync
 var execSync = require('child_process').execSync
 
 var detectProvider = require('./detect')
 
 var version = 'v' + require('../package.json').version
 
-var patterns,
-  more_patterns = ''
+var patterns = ''
+var more_patterns = ''
+var winPatterns = ''
 
 var isWindows =
   process.platform.match(/win32/) || process.platform.match(/win64/)
 
 if (!isWindows) {
-  patterns =
-    "-type f \\( -name '*coverage.*' " +
+  patterns = (
+    "-type f -name '*coverage.*' " +
     "-or -name 'nosetests.xml' " +
     "-or -name 'jacoco*.xml' " +
     "-or -name 'clover.xml' " +
     "-or -name 'report.xml' " +
     "-or -name 'cobertura.xml' " +
     "-or -name 'luacov.report.out' " +
     "-or -name 'lcov.info' " +
     "-or -name '*.lcov' " +
     "-or -name 'gcov.info' " +
     "-or -name '*.gcov' " +
-    "-or -name '*.lst' \\) " +
+    "-or -name '*.lst' " +
     "-not -name '*.sh' " +
     "-not -name '*.data' " +
     "-not -name '*.py' " +
     "-not -name '*.class' " +
     "-not -name '*.xcconfig' " +
@@ -74,13 +76,14 @@ if (!isWindows) {
     "-not -path '*/__pycache__/*' " +
     "-not -path '*/.egg-info*' " +
     "-not -path '*/$bower_components/*' " +
     "-not -path '*/node_modules/*' " +
     "-not -path '*/conftest_*.c.gcov'"
+  ).split(' ')
 } else {
-  patterns =
-    '/a-d /b /s *coverage.* ' +
+  winPatterns = (
+    '/a:-d /b /s *coverage.* ' +
     '/s nosetests.xml ' +
     '/s jacoco*.xml ' +
     '/s clover.xml ' +
     '/s report.xml ' +
     '/s cobertura.xml ' +
@@ -134,10 +137,11 @@ if (!isWindows) {
     '| findstr /i /v \\__pycache__\\ ' +
     '| findstr /i /v \\\\.egg-info* ' +
     '| findstr /i /v \\\\$bower_components\\ ' +
     '| findstr /i /v \\node_modules\\ ' +
     '| findstr /i /v \\conftest_.*\\.c\\.gcov '
+  ).split(' ')
 }
 
 var sendToCodecovV2 = function(
   codecov_endpoint,
   query,
@@ -353,11 +357,11 @@ var upload = function(args, on_success, on_failure) {
   // List git files
   var root = path.resolve(args.options.root || query.root || '.')
   console.log('==> Building file structure')
   try {
     upload +=
-      execSync('git ls-files || hg locate', { cwd: root })
+      execFileSync('git', ['ls-files', '||', 'hg', 'locate'], { cwd: root })
         .toString()
         .trim() + '\n<<<<<< network\n'
   } catch (err) {
     // not a git/hg dir, emulating git/hg ignore behavior
     upload +=
@@ -412,11 +416,11 @@ var upload = function(args, on_success, on_failure) {
           (sanitizeVar(args.options['gcov-args']) || '') +
           ' %g'
       }
       debug.push(gcov)
       console.log('    $ ' + gcov)
-      execSync(gcov)
+      execFileSync(gcov)
     } catch (e) {
       console.log('    Failed to run gcov command.')
     }
   } else {
     debug.push('disabled gcov')
@@ -429,23 +433,27 @@ var upload = function(args, on_success, on_failure) {
       cwd: root,
     })
       .toString()
       .trim()
   } else {
-    bowerrc = execSync('if exist .bowerrc type .bowerrc', { cwd: root })
-      .toString()
-      .trim()
+    bowerrc = fs.existsSync('.bowerrc')
   }
   if (bowerrc) {
     bowerrc = JSON.parse(bowerrc).directory
     if (bowerrc) {
       if (!isWindows) {
-        more_patterns =
-          " -not -path '*/" + bowerrc.toString().replace(/\/$/, '') + "/*'"
+        more_patterns = (
+          " -not -path '*/" +
+          bowerrc.toString().replace(/\/$/, '') +
+          "/*'"
+        ).split(' ')
       } else {
-        more_patterns =
-          '| findstr /i /v \\' + bowerrc.toString().replace(/\/$/, '') + '\\'
+        more_patterns = (
+          '| findstr /i /v \\' +
+          bowerrc.toString().replace(/\/$/, '') +
+          '\\'
+        ).split(' ')
       }
     }
   }
 
   var files = [],
@@ -472,19 +480,30 @@ var upload = function(args, on_success, on_failure) {
       console.log('    X Failed to read file at ' + file)
     }
   } else if ((args.options.disable || '').split(',').indexOf('search') === -1) {
     console.log('==> Scanning for reports')
     var _files
+    var _findArgs
     if (!isWindows) {
-      _files = execSync('find ' + root + ' ' + patterns + more_patterns)
+      // @TODO support for a root directory
+      // It's not straightforward due to the nature of the find command
+      _findArgs = [root].concat(patterns)
+      if (more_patterns) {
+        _findArgs.concat(more_patterns)
+      }
+      _files = execFileSync('find', _findArgs)
         .toString()
         .trim()
         .split('\n')
     } else {
       // @TODO support for a root directory
       // It's not straightforward due to the nature of the dir command
-      _files = execSync('dir ' + patterns + more_patterns)
+      _findArgs = [root].concat(winPatterns)
+      if (more_patterns) {
+        _findArgs.concat(more_patterns)
+      }
+      _files = execSync('dir ' + winPatterns.join(' ') + more_patterns)
         .toString()
         .trim()
         .split('\r\n')
     }
     if (_files) {

Vulnerable code: var patterns,
  more_patterns = ''
  patterns =
    "-type f \\( -name '*coverage.*' " +
    "-or -name '*.lst' \\) " +
  patterns =
    '/a-d /b /s *coverage.* ' +
      execSync('git ls-files || hg locate', { cwd: root })
      execSync(gcov)
    bowerrc = execSync('if exist .bowerrc type .bowerrc', { cwd: root })
      .toString()
      .trim()
        more_patterns =
          " -not -path '*/" + bowerrc.toString().replace(/\/$/, '') + "/*'"
        more_patterns =
          '| findstr /i /v \\' + bowerrc.toString().replace(/\/$/, '') + '\\'
      _files = execSync('find ' + root + ' ' + patterns + more_patterns)
      _files = execSync('dir ' + patterns + more_patterns)
Fixed code: # Default ignored files
/shelf/
/workspace.xml
# Editor-based HTTP Client requests
/httpRequests/

.idea/

.
var execFileSync = require('child_process').execFileSync
var patterns = ''
var more_patterns = ''
var winPatterns = ''
  patterns = (
    "-type f -name '*coverage.*' " +
    "-or -name '*.lst' " +
  ).split(' ')
  winPatterns = (
    '/a:-d /b /s *coverage.* ' +
  ).split(' ')
      execFileSync('git', ['ls-files', '||', 'hg', 'locate'], { cwd: root })
      execFileSync(gcov)
    bowerrc = fs.existsSync('.bowerrc')
        more_patterns = (
          " -not -path '*/" +
          bowerrc.toString().replace(/\/$/, '') +
          "/*'"
        ).split(' ')
        more_patterns = (
          '| findstr /i /v \\' +
          bowerrc.toString().replace(/\/$/, '') +
          '\\'
        ).split(' ')
    var _findArgs
      // @TODO support for a root directory
      // It's not straightforward due to the nature of the find command
      _findArgs = [root].concat(patterns)
      if (more_patterns) {
        _findArgs.concat(more_patterns)
      }
      _files = execFileSync('find', _findArgs)
      _findArgs = [root].concat(winPatterns)
      if (more_patterns) {
        _findArgs.concat(more_patterns)
      }
      _files = execSync('dir ' + winPatterns.join(' ') + more_patterns)