Merge pull request from GHSA-7xcx-6wjh-7xp2

- Uses `execFile` with arguments instead of `exec` where possible.
- See GHSL-2020-111

Co-authored-by: Benjamin E. Coe <bencoe@google.com>

lib/lifecycles/commit.js

@@ -2,7 +2,7 @@ const bump = require('../lifecycles/bump')
 const checkpoint = require('../checkpoint')
 const formatCommitMessage = require('../format-commit-message')
 const path = require('path')
-const runExec = require('../run-exec')
+const runExecFile = require('../run-execFile')
 const runLifecycleScript = require('../run-lifecycle-script')
 
 module.exports = function (args, newVersion) {
@@ -20,20 +20,21 @@ module.exports = function (args, newVersion) {
 function execCommit (args, newVersion) {
   let msg = 'committing %s'
   let paths = []
-  const verify = args.verify === false || args.n ? '--no-verify ' : ''
-  let toAdd = ''
+  const verify = args.verify === false || args.n ? ['--no-verify'] : []
+  const sign = args.sign ? ['-S'] : []
+  const toAdd = []
 
   // only start with a pre-populated paths list when CHANGELOG processing is not skipped
   if (!args.skip.changelog) {
     paths = [args.infile]
-    toAdd += ' ' + args.infile
+    toAdd.push(args.infile)
   }
 
   // commit any of the config files that we've updated
   // the version # for.
   Object.keys(bump.getUpdatedConfigs()).forEach(function (p) {
     paths.unshift(p)
-    toAdd += ' ' + path.relative(process.cwd(), p)
+    toAdd.push(path.relative(process.cwd(), p))
 
     // account for multiple files in the output message
     if (paths.length > 1) {
@@ -53,8 +54,22 @@ function execCommit (args, newVersion) {
     return Promise.resolve()
   }
 
-  return runExec(args, 'git add' + toAdd)
+  return runExecFile(args, 'git', ['add'].concat(toAdd))
     .then(() => {
-      return runExec(args, 'git commit ' + verify + (args.sign ? '-S ' : '') + (args.commitAll ? '' : (toAdd)) + ' -m "' + formatCommitMessage(args.releaseCommitMessageFormat, newVersion) + '"')
+      return runExecFile(
+        args,
+        'git',
+        [
+          'commit'
+        ].concat(
+          verify,
+          sign,
+          args.commitAll ? [] : toAdd,
+          [
+            '-m',
+            `"${formatCommitMessage(args.releaseCommitMessageFormat, newVersion)}"`
+          ]
+        )
+      )
     })
 }

lib/lifecycles/tag.js

@@ -3,7 +3,7 @@ const chalk = require('chalk')
 const checkpoint = require('../checkpoint')
 const figures = require('figures')
 const formatCommitMessage = require('../format-commit-message')
-const runExec = require('../run-exec')
+const runExecFile = require('../run-execFile')
 const runLifecycleScript = require('../run-lifecycle-script')
 
 module.exports = function (newVersion, pkgPrivate, args) {
@@ -20,13 +20,13 @@ module.exports = function (newVersion, pkgPrivate, args) {
 function execTag (newVersion, pkgPrivate, args) {
   let tagOption
   if (args.sign) {
-    tagOption = '-s '
+    tagOption = '-s'
   } else {
-    tagOption = '-a '
+    tagOption = '-a'
   }
   checkpoint(args, 'tagging release %s%s', [args.tagPrefix, newVersion])
-  return runExec(args, 'git tag ' + tagOption + args.tagPrefix + newVersion + ' -m "' + formatCommitMessage(args.releaseCommitMessageFormat, newVersion) + '"')
-    .then(() => runExec('', 'git rev-parse --abbrev-ref HEAD'))
+  return runExecFile(args, 'git', ['tag', tagOption, args.tagPrefix + newVersion, '-m', `"${formatCommitMessage(args.releaseCommitMessageFormat, newVersion)}"`])
+    .then(() => runExecFile('', 'git', ['rev-parse', '--abbrev-ref', 'HEAD']))
     .then((currentBranch) => {
       let message = 'git push --follow-tags origin ' + currentBranch.trim()
       if (pkgPrivate !== true && bump.getUpdatedConfigs()['package.json']) {

lib/run-execFile.js

@@ -0,0 +1,20 @@
+const { execFile } = require('child_process')
+const printError = require('./print-error')
+
+module.exports = function (args, cmd, cmdArgs) {
+  if (args.dryRun) return Promise.resolve()
+  return new Promise((resolve, reject) => {
+    // Exec given cmd and handle possible errors
+    execFile(cmd, cmdArgs, function (err, stdout, stderr) {
+      // If exec returns content in stderr, but no error, print it as a warning
+      // If exec returns an error, print it and exit with return code 1
+      if (err) {
+        printError(args, stderr || err.message)
+        return reject(err)
+      } else if (stderr) {
+        printError(args, stderr, { level: 'warn', color: 'yellow' })
+      }
+      return resolve(stdout)
+    })
+  })
+}

test.js

@@ -258,9 +258,10 @@ describe('cli', function () {
           const captured = shell.cat('gitcapture.log').stdout.split('\n').map(function (line) {
             return line ? JSON.parse(line) : line
           })
-          captured[captured.length - 4].should.deep.equal(['commit', '-S', 'CHANGELOG.md', 'package.json', '-m', 'chore(release): 1.0.1'])
-          captured[captured.length - 3].should.deep.equal(['tag', '-s', 'v1.0.1', '-m', 'chore(release): 1.0.1'])
-
+          /* eslint-disable no-useless-escape */
+          captured[captured.length - 4].should.deep.equal(['commit', '-S', 'CHANGELOG.md', 'package.json', '-m', '\"chore(release): 1.0.1\"'])
+          captured[captured.length - 3].should.deep.equal(['tag', '-s', 'v1.0.1', '-m', '\"chore(release): 1.0.1\"'])
+          /* eslint-enable no-useless-escape */
           unmock()
         })
     })
@@ -1314,3 +1315,19 @@ describe('standard-version', function () {
     })
   })
 })
+
+describe('GHSL-2020-111', function () {
+  beforeEach(initInTempFolder)
+  afterEach(finishTemp)
+  it('does not allow command injection via basic configuration', function () {
+    return standardVersion({
+      silent: true,
+      noVerify: true,
+      infile: 'foo.txt',
+      releaseCommitMessageFormat: 'bla `touch exploit`'
+    }).then(function () {
+      const stat = shell.test('-f', './exploit')
+      stat.should.equal(false)
+    })
+  })
+})