From 5f5c77490aa84ed313405c88905eb4566135be31 Mon Sep 17 00:00:00 2001
From: Rowan Wookey <admin@rwky.net>
Date: Mon, 26 Oct 2015 18:56:09 +0000
Subject: [PATCH] Fixed security issue with gm.compare where arguments aren't
 being escaped properly

---
 History.md       |  4 ++++
 lib/compare.js   | 54 ++++++++++++++++++++++++++++--------------------
 lib/composite.js |  2 --
 lib/montage.js   |  2 --
 package.json     |  2 +-
 5 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/History.md b/History.md
index 70d32a7c..ae2a467f 100644
--- a/History.md
+++ b/History.md
@@ -1,3 +1,7 @@
+1.21.0 / 2015-10-26 **contains security fix**
+
+* fixed: gm.compare fails to escape arguments properly (Reported by Brendan Scarvell) [rwky](https://github.com/rwky)
+
 1.20.0 / 2015-09-23
 
 * changed: Reverted "Add format inference from filename for buffers/streams" due to errors #448
diff --git a/lib/compare.js b/lib/compare.js
index 07a56af2..35c8d51e 100644
--- a/lib/compare.js
+++ b/lib/compare.js
@@ -1,7 +1,6 @@
 // compare
 
-var exec = require('child_process').exec;
-var utils = require('./utils');
+var spawn = require('child_process').spawn;
 
 /**
  * Compare two images uses graphicsmagicks `compare` command.
@@ -20,13 +19,11 @@ var utils = require('./utils');
 
 module.exports = exports = function (proto) {
   function compare(orig, compareTo, options, cb) {
-    orig = utils.escape(orig);
-    compareTo = utils.escape(compareTo);
 
     var isImageMagick = this._options && this._options.imageMagick;
     // compare binary for IM is `compare`, for GM it's `gm compare`
     var bin = isImageMagick ? '' : 'gm ';
-    var execCmd = bin + 'compare -metric mse ' + orig + ' ' + compareTo;
+    var args = ['compare', '-metric', 'mse', orig, compareTo]
     var tolerance = 0.4;
     // outputting the diff image
     if (typeof options === 'object') {
@@ -40,16 +37,19 @@ module.exports = exports = function (proto) {
           throw new TypeError('The path for the diff output is invalid');
         }
          // graphicsmagick defaults to red
-        var highlightColorOption = options.highlightColor
-          ? ' -highlight-color ' + options.highlightColor + ' '
-          : ' ';
-        var highlightStyleOption = options.highlightStyle
-          ? ' -highlight-style ' + options.highlightStyle + ' '
-          : ' ';
-        var diffFilename = utils.escape(options.file);
+        if (options.highlightColour) {
+            args.push('-highlight-color');
+            args.push(options.highlightColor);
+        }
+        if (options.highlightStyle) {
+            args.push('-highlight-style')
+            args.push(options.highlightStyle)
+        }
         // For IM, filename is the last argument. For GM it's `-file <filename>`
-        var diffOpt = isImageMagick ? diffFilename : ('-file ' + diffFilename);
-        execCmd += highlightColorOption + highlightStyleOption  + ' ' + diffOpt;
+        if (isImageMagick) {
+            args.push('-file');
+        }
+        args.push(options.file);
       }
       
       if (typeof options.tolerance != 'undefined') {
@@ -60,7 +60,9 @@ module.exports = exports = function (proto) {
       } 
     } else {
       // For ImageMagick diff file is required but we don't care about it, so null it out
-      isImageMagick && (execCmd += ' null:');
+      if (isImageMagick) {
+        args.push('null:');
+      }
 
       if (typeof options == 'function') {
         cb = options; // tolerance value not provided, flip the cb place
@@ -69,19 +71,27 @@ module.exports = exports = function (proto) {
       }
     }
 
-    exec(execCmd, function (err, stdout, stderr) {
+    var proc = spawn('/usr/bin/gm', args);
+    var stdout = '';
+    var stderr = '';
+    proc.stdout.on('data',function(data) { stdout+=data });
+    proc.stderr.on('data',function(data) { stderr+=data });
+    proc.on('close', function (code) {
       // ImageMagick returns err code 2 if err, 0 if similar, 1 if dissimilar
       if (isImageMagick) {
-        if (!err) {
+        if (code === 0) {
           return cb(null, 0 <= tolerance, 0, stdout);
         }
-        if (err.code === 1) {
+        else if (code === 1) {
           err = null;
           stdout = stderr;
+        } else {
+        return cb(stderr);
+        }
+      } else {
+        if(code !== 0) {
+          return cb(stderr);
         }
-      }
-      if (err) {
-        return cb(err);
       }
       // Since ImageMagick similar gives err code 0 and no stdout, there's really no matching
       // Otherwise, output format for IM is `12.00 (0.123)` and for GM it's `Total: 0.123`
@@ -93,7 +103,7 @@ module.exports = exports = function (proto) {
       }
 
       var equality = parseFloat(match[1]);
-      cb(null, equality <= tolerance, equality, stdout, utils.unescape(orig), utils.unescape(compareTo));
+      cb(null, equality <= tolerance, equality, stdout, orig, compareTo);
     });
   }
 
diff --git a/lib/composite.js b/lib/composite.js
index 069a3b94..ca90440f 100644
--- a/lib/composite.js
+++ b/lib/composite.js
@@ -1,7 +1,5 @@
 // composite
 
-var utils = require('./utils');
-
 /**
  * Composite images together using the `composite` command in graphicsmagick.
  *
diff --git a/lib/montage.js b/lib/montage.js
index 8e9ccc10..3120be84 100644
--- a/lib/montage.js
+++ b/lib/montage.js
@@ -1,7 +1,5 @@
 // montage
 
-var utils = require('./utils');
-
 /**
  * Montage images next to each other using the `montage` command in graphicsmagick.
  *
diff --git a/package.json b/package.json
index 02e94672..200f9c71 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "gm",
   "description": "GraphicsMagick and ImageMagick for node.js",
-  "version": "1.20.0",
+  "version": "1.21.0",
   "author": "Aaron Heckmann <aaron.heckmann+github@gmail.com>",
   "keywords": [
     "graphics",