CVE-2020-4059: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: In mversion before 2.0.0, there is a command injection vulnerability. This issue may lead to remote code execution if a client of the library calls the vulnerable method with untrusted input. This vulnerability is patched by version 2.0.0. Previous releases are deprecated in npm. As a workaround, make sure to escape git commit messages when using the commitMessage option for the update function.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 7.3
Published: 2020-06-18T20:15Z
Repository: https://github.com/mikaelbr/mversion
Language: JavaScript
Files changed: lib/git.js, tests/git_test.js
Commit date: 2020-06-15 18:28:53 +0200
Commit message: Fixes missing shell escape for git commit message

Diff: diff --git a/lib/git.js b/lib/git.js
index b4e1542..acf6297 100644
--- a/lib/git.js
+++ b/lib/git.js
@@ -1,62 +1,71 @@
-var contra = require('contra'),
-    path = require('path'),
-    fUtils = require('./files'),
-    cp = require('child_process');
-
-var gitApp = 'git', gitExtra = { env: process.env };
+var contra = require("contra"),
+  path = require("path"),
+  fUtils = require("./files"),
+  cp = require("child_process");
 
+var gitApp = "git",
+  gitExtra = { env: process.env };
 
 var escapeQuotes = function (str) {
-  if (typeof str === 'string') {
-    return str.replace(/(["$`\\])/g, '\\$1');
+  if (typeof str === "string") {
+    return '"' + str.replace(/(["'$`\\])/g, "\\$1") + '"';
   } else {
     return str;
   }
 };
 
 module.exports.isRepositoryClean = function (callback) {
-  cp.exec(gitApp + ' ' + [ 'ls-files', '-m' ].join(' '), gitExtra, function (er, stdout, stderr) {
+  cp.exec(gitApp + " " + ["ls-files", "-m"].join(" "), gitExtra, function (
+    er,
+    stdout,
+    stderr
+  ) {
     // makeCommit parly inspired and taken from NPM version module
-    var lines = stdout.trim().split('\n').filter(function (line) {
-      var file = path.basename(line.replace(/.{1,2}\s+/, ''));
-      return line.trim() && !line.match(/^\?\? /) && !fUtils.isPackageFile(line);
-    }).map(function (line) {
-      return line.trim()
-    });
+    var lines = stdout
+      .trim()
+      .split("\n")
+      .filter(function (line) {
+        var file = path.basename(line.replace(/.{1,2}\s+/, ""));
+        return (
+          line.trim() && !line.match(/^\?\? /) && !fUtils.isPackageFile(line)
+        );
+      })
+      .map(function (line) {
+        return line.trim();
+      });
 
     if (lines.length) {
-      return callback(new Error('Git working directory not clean.\n'+lines.join('\n')));
+      return callback(
+        new Error("Git working directory not clean.\n" + lines.join("\n"))
+      );
     }
     return callback();
   });
 };
 
 module.exports.checkout = function (callback) {
-  cp.exec(gitApp + ' checkout -- .', gitExtra, callback);
+  cp.exec(gitApp + " checkout -- .", gitExtra, callback);
 };
 
 module.exports.commit = function (files, message, newVer, tagName, callback) {
-  message = message.replace('%s', newVer).replace('"', '').replace("'", '');
-  files = files.map(function (file) {
-    return '"' + escapeQuotes(file) + '"';
-  }).join(' ');
+  message = escapeQuotes(message.replace("%s", newVer));
+  files = files.map(escapeQuotes).join(" ");
   var functionSeries = [
     function (done) {
-      cp.exec(gitApp + ' add ' + files, gitExtra, done);
+      cp.exec(gitApp + " add " + files, gitExtra, done);
     },
 
     function (done) {
-      cp.exec([gitApp, 'commit', '-m', '"' + message + '"'].join(' '), gitExtra, done);
+      cp.exec([gitApp, "commit", "-m", message].join(" "), gitExtra, done);
     },
 
     function (done) {
       cp.exec(
-        [
-          gitApp, 'tag', '-a', tagName, '-m', '"' + message + '"'
-        ].join(' '),
-        gitExtra, done
+        [gitApp, "tag", "-a", tagName, "-m", message].join(" "),
+        gitExtra,
+        done
       );
-    }
+    },
   ];
   contra.series(functionSeries, callback);
 };
diff --git a/tests/git_test.js b/tests/git_test.js
index fce571a..52ad44f 100644
--- a/tests/git_test.js
+++ b/tests/git_test.js
@@ -1,41 +1,41 @@
-var version = require('../'),
-    assert = require('assert'),
-    fs = require('fs'),
-    vinylFs = require('vinyl-fs'),
-    path = require('path'),
-    cp = require('child_process'),
-    File = require('vinyl'),
-    through = require('through2'),
-    fUtil = require('../lib/files'),
-    git = require('../lib/git');
-
-describe('git', function () {
-  var filename = 'package.json';
-  var expectedPath = path.join(__dirname, './fixtures/', filename);
+var version = require("../"),
+  assert = require("assert"),
+  fs = require("fs"),
+  vinylFs = require("vinyl-fs"),
+  path = require("path"),
+  cp = require("child_process"),
+  File = require("vinyl"),
+  through = require("through2"),
+  fUtil = require("../lib/files"),
+  git = require("../lib/git");
+
+describe("git", function () {
+  var filename = "package.json";
+  var expectedPath = path.join(__dirname, "./fixtures/", filename);
   var expectedContent = fs.readFileSync(expectedPath);
 
   var original = fUtil.loadFiles;
   var dest = vinylFs.dest;
   var exec = cp.exec;
   var originalIsRepositoryClean = git.isRepositoryClean;
   var originalCommit = git.commit;
   var originalCheckout = git.checkout;
 
-  before(function () {
+  before(function () {
     vinylFs.dest = function () {
       return through.obj(function (file, enc, next) {
         this.push(file);
         next();
       });
-    }
+    };
 
     var expectedFile = new File({
       base: __dirname,
       cwd: __dirname,
       path: expectedPath,
-      contents: expectedContent
+      contents: expectedContent,
     });
 
     fUtil.loadFiles = function () {
       var stream = through.obj();
       stream.write(expectedFile);
@@ -55,132 +55,164 @@ describe('git', function () {
     git.checkout = originalCheckout;
 
     cp.exec = exec;
   });
 
-  describe('#Update()', function(){
-    it('should return error on unclean git repository when commit is given', function (done) {
+  describe("#Update()", function () {
+    it("should return error on unclean git repository when commit is given", function (done) {
       git.isRepositoryClean = function (cb) {
-        return cb(new Error('Not clean'));
+        return cb(new Error("Not clean"));
       };
 
-      version.update({
-        version: '1.0.0',
-        commitMessage: 'Message'
-      }, function (err, data) {
-        assert.ok(err);
-        assert.equal(err.message, 'Not clean', 'Error message should be set by isRepositoryClean');
+      version.update(
+        {
+          version: "1.0.0",
+          commitMessage: "Message",
+        },
+        function (err, data) {
+          assert.ok(err);
+          assert.equal(
+            err.message,
+            "Not clean",
+            "Error message should be set by isRepositoryClean"
+          );
+
+          done();
+        }
+      );
+    });
 
+    it("should return NOT error on unclean git repository when no commit message is given", function (done) {
+      git.isRepositoryClean = function (cb) {
+        return cb(new Error("Not clean"));
+      };
+
+      version.update("1.0.0", function (err, data) {
+        assert.ifError(err);
         done();
       });
     });
 
-    it('should return NOT error on unclean git repository when no commit message is given', function (done) {
+    it("should sanitize commit message", function (done) {
       git.isRepositoryClean = function (cb) {
-        return cb(new Error('Not clean'));
+        return cb(null);
       };
 
-      version.update('1.0.0', function (err, data) {
-        assert.ifError(err);
+      cp.exec = function (cmd, extra, cb) {
+        if (cmd.indexOf("-a") === -1) return cb(null);
+        assert.equal('git tag -a v1.0.0 -m "Message \\`touch file\\`"', cmd);
         done();
+      };
+
+      version.update({
+        version: "1.0.0",
+        commitMessage: "Message `touch file`",
       });
     });
 
-    it('should get updated version sent to commit when commit message is given', function (done) {
+    it("should get updated version sent to commit when commit message is given", function (done) {
       git.isRepositoryClean = function (cb) {
         return cb(null);
       };
 
       git.commit = function (files, message, newVer, tagName, callback) {
-        assert.equal(message, 'Message');
-        assert.equal(newVer, '1.0.0');
+        assert.equal(message, "Message");
+        assert.equal(newVer, "1.0.0");
         assert.equal(files[0], expectedPath);
-        assert.equal(tagName, 'v1.0.0');
+        assert.equal(tagName, "v1.0.0");
         return callback(null);
       };
 
-      version.update({
-        version: '1.0.0',
-        commitMessage: 'Message'
-      }, function (err, data) {
-        assert.ifError(err);
-        done();
-      });
+      version.update(
+        {
+          version: "1.0.0",
+          commitMessage: "Message",
+        },
+        function (err, data) {
+          assert.ifError(err);
+          done();
+        }
+      );
     });
 
-    it('should be able to override tagName', function (done) {
+    it("should be able to override tagName", function (done) {
       git.isRepositoryClean = function (cb) {
         return cb(null);
       };
 
       git.commit = function (files, message, newVer, tagName, callback) {
-        assert.equal(tagName, 'v1.0.0-src');
+        assert.equal(tagName, "v1.0.0-src");
         return callback(null);
       };
 
-      version.update({
-        version: '1.0.0',
-        commitMessage: 'Message',
-        tagName: 'v%s-src'
-      }, function (err, data) {
-        assert.ifError(err);
-        done();
-      });
+      version.update(
+        {
+          version: "1.0.0",
+          commitMessage: "Message",
+          tagName: "v%s-src",
+        },
+        function (err, data) {
+          assert.ifError(err);
+          done();
+        }
+      );
     });
 
-    it('should get flag defining if v-prefix should be used or not', function (done) {
+    it("should get flag defining if v-prefix should be used or not", function (done) {
       git.isRepositoryClean = function (cb) {
         return cb(null);
       };
 
       git.commit = function (files, message, newVer, noPrefix, callback) {
-        assert.ok(noPrefix, 'No prefix should be true');
+        assert.ok(noPrefix, "No prefix should be true");
         return callback(null);
       };
 
-      version.update({
-        version: '1.0.0',
-        commitMessage: 'Message',
-        noPrefix: true
-      }, function (err, data) {
-        assert.ifError(err);
-        done();
-      });
+      version.update(
+        {
+          version: "1.0.0",
+          commitMessage: "Message",
+          noPrefix: true,
+        },
+        function (err, data) {
+          assert.ifError(err);
+          done();
+        }
+      );
     });
 
-    it('should make tag with v-prefix per default', function (done) {
+    it("should make tag with v-prefix per default", function (done) {
       git.isRepositoryClean = function (cb) {
         return cb(null);
       };
 
       cp.exec = function (cmd, extra, cb) {
-        if (cmd.indexOf('-a') === -1) return cb(null);
+        if (cmd.indexOf("-a") === -1) return cb(null);
         assert.equal('git tag -a v1.0.0 -m "Message"', cmd);
         done();
       };
 
       version.update({
-        version: '1.0.0',
-        commitMessage: 'Message'
+        version: "1.0.0",
+        commitMessage: "Message",
       });
     });
 
-    it('should make tag without v-prefix if specified', function (done) {
+    it("should make tag without v-prefix if specified", function (done) {
       git.isRepositoryClean = function (cb) {
         return cb(null);
       };
 
       cp.exec = function (cmd, extra, cb) {
-        if (cmd.indexOf('-a') === -1) return cb(null);
+        if (cmd.indexOf("-a") === -1) return cb(null);
         assert.equal('git tag -a 1.0.0 -m "Message"', cmd);
         done();
       };
 
       version.update({
-        version: '1.0.0',
-        commitMessage: 'Message',
-        noPrefix: true
+        version: "1.0.0",
+        commitMessage: "Message",
+        noPrefix: true,
       });
     });
   });
-
-});
\ No newline at end of file
+});

Vulnerable code: var contra = require('contra'),
    path = require('path'),
    fUtils = require('./files'),
    cp = require('child_process');

var gitApp = 'git', gitExtra = { env: process.env };
  if (typeof str === 'string') {
    return str.replace(/(["$`\\])/g, '\\$1');
  cp.exec(gitApp + ' ' + [ 'ls-files', '-m' ].join(' '), gitExtra, function (er, stdout, stderr) {
    var lines = stdout.trim().split('\n').filter(function (line) {
      var file = path.basename(line.replace(/.{1,2}\s+/, ''));
      return line.trim() && !line.match(/^\?\? /) && !fUtils.isPackageFile(line);
    }).map(function (line) {
      return line.trim()
    });
      return callback(new Error('Git working directory not clean.\n'+lines.join('\n')));
  cp.exec(gitApp + ' checkout -- .', gitExtra, callback);
  message = message.replace('%s', newVer).replace('"', '').replace("'", '');
  files = files.map(function (file) {
    return '"' + escapeQuotes(file) + '"';
  }).join(' ');
      cp.exec(gitApp + ' add ' + files, gitExtra, done);
      cp.exec([gitApp, 'commit', '-m', '"' + message + '"'].join(' '), gitExtra, done);
        [
          gitApp, 'tag', '-a', tagName, '-m', '"' + message + '"'
        ].join(' '),
        gitExtra, done
    }
var version = require('../'),
    assert = require('assert'),
    fs = require('fs'),
    vinylFs = require('vinyl-fs'),
    path = require('path'),
    cp = require('child_process'),
    File = require('vinyl'),
    through = require('through2'),
    fUtil = require('../lib/files'),
    git = require('../lib/git');

describe('git', function () {
  var filename = 'package.json';
  var expectedPath = path.join(__dirname, './fixtures/', filename);
  before(function () {
    }
      contents: expectedContent
  describe('#Update()', function(){
    it('should return error on unclean git repository when commit is given', function (done) {
        return cb(new Error('Not clean'));
      version.update({
        version: '1.0.0',
        commitMessage: 'Message'
      }, function (err, data) {
        assert.ok(err);
        assert.equal(err.message, 'Not clean', 'Error message should be set by isRepositoryClean');
    it('should return NOT error on unclean git repository when no commit message is given', function (done) {
        return cb(new Error('Not clean'));
      version.update('1.0.0', function (err, data) {
        assert.ifError(err);
    it('should get updated version sent to commit when commit message is given', function (done) {
        assert.equal(message, 'Message');
        assert.equal(newVer, '1.0.0');
        assert.equal(tagName, 'v1.0.0');
      version.update({
        version: '1.0.0',
        commitMessage: 'Message'
      }, function (err, data) {
        assert.ifError(err);
        done();
      });
    it('should be able to override tagName', function (done) {
        assert.equal(tagName, 'v1.0.0-src');
      version.update({
        version: '1.0.0',
        commitMessage: 'Message',
        tagName: 'v%s-src'
      }, function (err, data) {
        assert.ifError(err);
        done();
      });
    it('should get flag defining if v-prefix should be used or not', function (done) {
        assert.ok(noPrefix, 'No prefix should be true');
      version.update({
        version: '1.0.0',
        commitMessage: 'Message',
        noPrefix: true
      }, function (err, data) {
        assert.ifError(err);
        done();
      });
    it('should make tag with v-prefix per default', function (done) {
        if (cmd.indexOf('-a') === -1) return cb(null);
        version: '1.0.0',
        commitMessage: 'Message'
    it('should make tag without v-prefix if specified', function (done) {
        if (cmd.indexOf('-a') === -1) return cb(null);
        version: '1.0.0',
        commitMessage: 'Message',
        noPrefix: true

});
Fixed code: var contra = require("contra"),
  path = require("path"),
  fUtils = require("./files"),
  cp = require("child_process");
var gitApp = "git",
  gitExtra = { env: process.env };
  if (typeof str === "string") {
    return '"' + str.replace(/(["'$`\\])/g, "\\$1") + '"';
  cp.exec(gitApp + " " + ["ls-files", "-m"].join(" "), gitExtra, function (
    er,
    stdout,
    stderr
  ) {
    var lines = stdout
      .trim()
      .split("\n")
      .filter(function (line) {
        var file = path.basename(line.replace(/.{1,2}\s+/, ""));
        return (
          line.trim() && !line.match(/^\?\? /) && !fUtils.isPackageFile(line)
        );
      })
      .map(function (line) {
        return line.trim();
      });
      return callback(
        new Error("Git working directory not clean.\n" + lines.join("\n"))
      );
  cp.exec(gitApp + " checkout -- .", gitExtra, callback);
  message = escapeQuotes(message.replace("%s", newVer));
  files = files.map(escapeQuotes).join(" ");
      cp.exec(gitApp + " add " + files, gitExtra, done);
      cp.exec([gitApp, "commit", "-m", message].join(" "), gitExtra, done);
        [gitApp, "tag", "-a", tagName, "-m", message].join(" "),
        gitExtra,
        done
    },
var version = require("../"),
  assert = require("assert"),
  fs = require("fs"),
  vinylFs = require("vinyl-fs"),
  path = require("path"),
  cp = require("child_process"),
  File = require("vinyl"),
  through = require("through2"),
  fUtil = require("../lib/files"),
  git = require("../lib/git");

describe("git", function () {
  var filename = "package.json";
  var expectedPath = path.join(__dirname, "./fixtures/", filename);
  before(function () {
    };
      contents: expectedContent,
  describe("#Update()", function () {
    it("should return error on unclean git repository when commit is given", function (done) {
        return cb(new Error("Not clean"));
      version.update(
        {
          version: "1.0.0",
          commitMessage: "Message",
        },
        function (err, data) {
          assert.ok(err);
          assert.equal(
            err.message,
            "Not clean",
            "Error message should be set by isRepositoryClean"
          );

          done();
        }
      );
    });
    it("should return NOT error on unclean git repository when no commit message is given", function (done) {
      git.isRepositoryClean = function (cb) {
        return cb(new Error("Not clean"));
      };

      version.update("1.0.0", function (err, data) {
        assert.ifError(err);
    it("should sanitize commit message", function (done) {
        return cb(null);
      cp.exec = function (cmd, extra, cb) {
        if (cmd.indexOf("-a") === -1) return cb(null);
        assert.equal('git tag -a v1.0.0 -m "Message \\`touch file\\`"', cmd);
      };

      version.update({
        version: "1.0.0",
        commitMessage: "Message `touch file`",
    it("should get updated version sent to commit when commit message is given", function (done) {
        assert.equal(message, "Message");
        assert.equal(newVer, "1.0.0");
        assert.equal(tagName, "v1.0.0");
      version.update(
        {
          version: "1.0.0",
          commitMessage: "Message",
        },
        function (err, data) {
          assert.ifError(err);
          done();
        }
      );
    it("should be able to override tagName", function (done) {
        assert.equal(tagName, "v1.0.0-src");
      version.update(
        {
          version: "1.0.0",
          commitMessage: "Message",
          tagName: "v%s-src",
        },
        function (err, data) {
          assert.ifError(err);
          done();
        }
      );
    it("should get flag defining if v-prefix should be used or not", function (done) {
        assert.ok(noPrefix, "No prefix should be true");
      version.update(
        {
          version: "1.0.0",
          commitMessage: "Message",
          noPrefix: true,
        },
        function (err, data) {
          assert.ifError(err);
          done();
        }
      );
    it("should make tag with v-prefix per default", function (done) {
        if (cmd.indexOf("-a") === -1) return cb(null);
        version: "1.0.0",
        commitMessage: "Message",
    it("should make tag without v-prefix if specified", function (done) {
        if (cmd.indexOf("-a") === -1) return cb(null);
        version: "1.0.0",
        commitMessage: "Message",
        noPrefix: true,
});