CVE-2018-21268: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: The traceroute (aka node-traceroute) package through 1.0.0 for Node.js allows remote command injection via the host parameter. This occurs because the Child.exec() method, which is considered to be not entirely safe, is used. In particular, an OS command can be placed after a newline character.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2020-06-25T17:15Z
Repository: https://github.com/jaw187/node-traceroute
Language: JSON
Files changed: package.json, traceroute.js
Commit date: 2016-03-10 21:42:39 -0500
Commit message: conversion to spawn and stream

Diff: diff --git a/package.json b/package.json
index dff2abe..6e4871d 100644
--- a/package.json
+++ b/package.json
@@ -8,14 +8,13 @@
     "url": "git://github.com/jaw187/node-traceroute.git"
   },
   "engines": {
     "node": ">=4.0.0"
   },
-  "dependencies": {},
   "devDependencies": {
     "code": "2.x.x",
-    "lab": "9.x.x"
+    "lab": "10.x.x"
   },
   "bugs": {
     "url": "https://github.com/jaw187/node-traceroute/issues"
   },
   "homepage": "https://github.com/jaw187/node-traceroute#readme",
diff --git a/traceroute.js b/traceroute.js
index a46a088..85c58b1 100644
--- a/traceroute.js
+++ b/traceroute.js
@@ -16,27 +16,47 @@ internals.isWin = /^win/.test(Os.platform());
 module.exports = internals.Traceroute = {};
 
 
 internals.Traceroute.trace = function (host, callback) {
 
-
     Dns.lookup(host.toUpperCase(), (err) => {
 
         if (err && Net.isIP(host) === 0) {
             return callback(new Error('Invalid host'));
         }
 
-        const command = (internals.isWin ? 'tracert -d ' : 'traceroute -q 1 -n ') + host;
-        Child.exec(command, (err, stdout, stderr) => {
+        const command = (internals.isWin ? 'tracert' : 'traceroute');
+        const args = internals.isWin ? ['-d', host] : ['-q', 1, '-n', host];
+
+        const traceroute = Child.spawn(command, args);
+
+        const hops = [];
+        let counter = 0;
+        traceroute.stdout.on('data', (data) => {
 
-            if (err) {
-                return callback(err);
+            ++counter;
+            if ((!internals.isWin && counter < 2) || (internals.isWin && counter < 5)) {
+                return null;
             }
 
-            const results = internals.parseOutput(stdout);
-            return callback(null, results);
+            const result = data.toString().replace(/\n$/,'');
+            if (!result) {
+                return null;
+            }
+
+            const hop = internals.parseHop(result);
+            hops.push(hop);
         });
+
+        traceroute.on('close', (code) => {
+
+            if (callback) {
+                return callback(null, hops);
+            }
+        });
+
+        return traceroute;
     });
 };
 
 
 internals.parseHop = function (hop) {
@@ -94,31 +114,5 @@ internals.parseHopNix = function (line) {
         }
     }
 
     return hop;
 };
-
-internals.parseOutput = function (output) {
-
-    const lines = output.split('\n');
-    const hops = [];
-
-    lines.shift();
-    lines.pop();
-
-    if (internals.isWin) {
-        for (let i = 0; i < lines.length; ++i) {
-            if (/^\s+1/.test(lines[i])) {
-                break;
-            }
-        }
-        lines.splice(0,i);
-        lines.pop();
-        lines.pop();
-    }
-
-    for (let i = 0; i < lines.length; ++i) {
-        hops.push(internals.parseHop(lines[i]));
-    }
-
-    return hops;
-};

Vulnerable code:   "dependencies": {},
    "lab": "9.x.x"

        const command = (internals.isWin ? 'tracert -d ' : 'traceroute -q 1 -n ') + host;
        Child.exec(command, (err, stdout, stderr) => {
            if (err) {
                return callback(err);
            const results = internals.parseOutput(stdout);
            return callback(null, results);

internals.parseOutput = function (output) {

    const lines = output.split('\n');
    const hops = [];

    lines.shift();
    lines.pop();

    if (internals.isWin) {
        for (let i = 0; i < lines.length; ++i) {
            if (/^\s+1/.test(lines[i])) {
                break;
            }
        }
        lines.splice(0,i);
        lines.pop();
        lines.pop();
    }

    for (let i = 0; i < lines.length; ++i) {
        hops.push(internals.parseHop(lines[i]));
    }

    return hops;
};
Fixed code:     "lab": "10.x.x"
        const command = (internals.isWin ? 'tracert' : 'traceroute');
        const args = internals.isWin ? ['-d', host] : ['-q', 1, '-n', host];

        const traceroute = Child.spawn(command, args);

        const hops = [];
        let counter = 0;
        traceroute.stdout.on('data', (data) => {
            ++counter;
            if ((!internals.isWin && counter < 2) || (internals.isWin && counter < 5)) {
                return null;
            const result = data.toString().replace(/\n$/,'');
            if (!result) {
                return null;
            }

            const hop = internals.parseHop(result);
            hops.push(hop);

        traceroute.on('close', (code) => {

            if (callback) {
                return callback(null, hops);
            }
        });

        return traceroute;