CVE-2020-28451: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: This affects the package image-tiler before 2.0.2.
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: 9.8
Published: 2022-08-02T14:15Z
Repository: https://github.com/MrP/image-tiler
Language: JavaScript
Files changed: index.js, spec/image-tiler.spec.js
Commit date: 2021-01-05 21:34:11 +0000
Commit message: fix command injection vuln

Diff: diff --git a/index.js b/index.js
index 4e8e6a7..f9a3d08 100644
--- a/index.js
+++ b/index.js
@@ -1,7 +1,7 @@
 'use strict';
-var execSync = require('child_process').execSync;
+var execFileSync = require('child_process').execFileSync;
 var sizeOf = require('image-size');
 var mkdirp = require('mkdirp-promise');
 var rimraf = require('rimraf-then');
 var fs = require('fs');
 var path = require('path');
@@ -12,43 +12,43 @@ function tileLevel(inPath, outPath, zoom, tileSize, pattern, quality) {
         .replace(/\{x\}/, '%[fx:page.x/' + tileSize + ']')
         .replace(/\{y\}/, '%[fx:page.y/' + tileSize + ']')
         .replace(/\.[^.]+$/, '');
     var patternedFilenameWithoutTheFilename = '';
     if (pattern.indexOf(path.sep) > 0) {
-        patternedFilenameWithoutTheFilename = pattern.replace(new RegExp(path.sep+'[^'+path.sep+']*$'), '')
-        .replace(/\{z\}/, '' + zoom);
+        patternedFilenameWithoutTheFilename = pattern.replace(new RegExp(path.sep + '[^' + path.sep + ']*$'), '')
+            .replace(/\{z\}/, '' + zoom);
     }
     return mkdirp(outPath + path.sep + patternedFilenameWithoutTheFilename)
-    .then(()=>{
-        var command = 'convert ' + inPath +
-            ' -crop ' + tileSize + 'x' + tileSize +
-            ' -set filename:tile "' + patternedFilename + '"' +
-            ' -quality ' + quality + ' +repage +adjoin' +
-            ' "' + outPath + '/%[filename:tile]' + dotExtension + '"' ;
-        execSync(command);
-    });
+        .then(() => {
+            var args = [inPath,
+                '-crop', tileSize + 'x' + tileSize,
+                '-set', 'filename:tile', patternedFilename,
+                '-quality', quality, '+repage', '+adjoin',
+                outPath + '/%[filename:tile]' + dotExtension];
+            execFileSync('convert', args);
+        });
 }
 
 function imageBiggerThanTile(path, tileSize) {
     var size = sizeOf(path);
     return size.height > tileSize || size.width > tileSize;
 }
 
 function tileRec(inPath, outPath, zoom, tileSize, tempDir, pattern, zoomToDisplay, invertZoom, quality) {
     var inPathMpc = tempDir + '/temp_level_' + zoom + '.mpc';
     var inPathCache = tempDir + '/temp_level_' + zoom + '.cache';
-    execSync('convert ' + inPath + ' ' + inPathMpc);
+    execFileSync('convert', [inPath, inPathMpc]);
     return tileLevel(inPathMpc, outPath, zoomToDisplay, tileSize, pattern, quality)
         .then(function () {
             if (imageBiggerThanTile(inPath, tileSize)) {
                 var newZoom = zoom + 1;
                 var newZoomToDisplay = zoomToDisplay + 1;
                 if (!invertZoom) {
                     newZoomToDisplay = zoomToDisplay - 1;
                 }
                 var newInPath = tempDir + '/temp_level_' + zoom + '.png';
-                execSync('convert ' + inPathMpc + ' -resize 50% -quality ' + quality + ' ' + newInPath);
+                execFileSync('convert', [inPathMpc, '-resize', '50%', '-quality', quality, newInPath]);
                 fs.unlinkSync(inPathMpc);
                 fs.unlinkSync(inPathCache);
                 return tileRec(newInPath, outPath, newZoom, tileSize, tempDir, pattern, newZoomToDisplay, invertZoom, quality);
             } else {
                 fs.unlinkSync(inPathMpc);
@@ -70,8 +70,8 @@ module.exports.tile = function (inPath, outPath, pattern, options) {
         var halvingsWidth = Math.ceil(Math.log2(Math.ceil(size.width / tileSize)));
         var halvingsheight = Math.ceil(Math.log2(Math.ceil(size.height / tileSize)));
         zoomToDisplay = Math.max(halvingsWidth, halvingsheight);
     }
     return mkdirp(tempDir)
-        .then(()=>tileRec(inPath, outPath, zoom, tileSize, tempDir, pattern, zoomToDisplay, options.invertZoom, quality))
-        .then(()=>rimraf(tempDir));
+        .then(() => tileRec(inPath, outPath, zoom, tileSize, tempDir, pattern, zoomToDisplay, options.invertZoom, quality))
+        .then(() => rimraf(tempDir));
 };
diff --git a/spec/image-tiler.spec.js b/spec/image-tiler.spec.js
index b96987b..5ae8294 100644
--- a/spec/image-tiler.spec.js
+++ b/spec/image-tiler.spec.js
@@ -1,8 +1,8 @@
 /*global jasmine*/
 var fs = require('fs');
-var execSync = require('child_process').execSync;
+var execFileSync = require('child_process').execFileSync;
 var rimraf = require('rimraf');
 var expectImagesToBeTheSame = require('./expectImagesToBeTheSame.helper.js').expectImagesToBeTheSame;
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 600000;
 var tmpDir = process.env.TMPDIR || '/tmp';
@@ -12,14 +12,14 @@ describe('image-tiler cli', function () {
     beforeEach(function () {
         fs.mkdirSync(tempDir);
     });
     describe('When used on an image smaller than the tile size', function () {
         it('should output the same image', function (done) {
-            execSync('node bin/image-tiler spec/small.png ' + tempDir + ' small_test_result_{z}_{x}_{y}.png');
+            execFileSync('node', ['bin/image-tiler', 'spec/small.png', tempDir, 'small_test_result_{z}_{x}_{y}.png']);
             expectImagesToBeTheSame(tempDir + '/small_test_result_0_0_0.png', 'spec/expected/small-test.png')
-            .then(done)
-            .catch(done.fail);
+                .then(done)
+                .catch(done.fail);
         });
     });
 
     afterEach(function () {
         rimraf.sync(tempDir);

Vulnerable code: var execSync = require('child_process').execSync;
        patternedFilenameWithoutTheFilename = pattern.replace(new RegExp(path.sep+'[^'+path.sep+']*$'), '')
        .replace(/\{z\}/, '' + zoom);
    .then(()=>{
        var command = 'convert ' + inPath +
            ' -crop ' + tileSize + 'x' + tileSize +
            ' -set filename:tile "' + patternedFilename + '"' +
            ' -quality ' + quality + ' +repage +adjoin' +
            ' "' + outPath + '/%[filename:tile]' + dotExtension + '"' ;
        execSync(command);
    });
    execSync('convert ' + inPath + ' ' + inPathMpc);
                execSync('convert ' + inPathMpc + ' -resize 50% -quality ' + quality + ' ' + newInPath);
        .then(()=>tileRec(inPath, outPath, zoom, tileSize, tempDir, pattern, zoomToDisplay, options.invertZoom, quality))
        .then(()=>rimraf(tempDir));
var execSync = require('child_process').execSync;
            execSync('node bin/image-tiler spec/small.png ' + tempDir + ' small_test_result_{z}_{x}_{y}.png');
            .then(done)
            .catch(done.fail);
Fixed code: var execFileSync = require('child_process').execFileSync;
        patternedFilenameWithoutTheFilename = pattern.replace(new RegExp(path.sep + '[^' + path.sep + ']*$'), '')
            .replace(/\{z\}/, '' + zoom);
        .then(() => {
            var args = [inPath,
                '-crop', tileSize + 'x' + tileSize,
                '-set', 'filename:tile', patternedFilename,
                '-quality', quality, '+repage', '+adjoin',
                outPath + '/%[filename:tile]' + dotExtension];
            execFileSync('convert', args);
        });
    execFileSync('convert', [inPath, inPathMpc]);
                execFileSync('convert', [inPathMpc, '-resize', '50%', '-quality', quality, newInPath]);
        .then(() => tileRec(inPath, outPath, zoom, tileSize, tempDir, pattern, zoomToDisplay, options.invertZoom, quality))
        .then(() => rimraf(tempDir));
var execFileSync = require('child_process').execFileSync;
            execFileSync('node', ['bin/image-tiler', 'spec/small.png', tempDir, 'small_test_result_{z}_{x}_{y}.png']);
                .then(done)
                .catch(done.fail);