CVE-2014-9682: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: The dns-sync module before 0.1.1 for node.js allows context-dependent attackers to execute arbitrary commands via shell metacharacters in the first argument to the resolve API 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.
Published: 2015-02-28T01:59Z
Repository: https://github.com/skoranga/node-dns-sync
Language: JavaScript
Files changed: lib/dns-sync.js, package.json, test/test.js
Commit date: 2014-11-10 15:44:07 -0800
Commit message: adding validation check for hostname

Diff: diff --git a/lib/dns-sync.js b/lib/dns-sync.js
index 5f63607..6c19af5 100644
--- a/lib/dns-sync.js
+++ b/lib/dns-sync.js
@@ -4,19 +4,31 @@ var net = require('net'),
     util = require('util'),
     path = require('path'),
     shell = require('shelljs'),
     debug = require('debug')('dns-sync');
 
+//source - http://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
+var ValidHostnameRegex = new RegExp("^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$");
+
+function isValidHostName(hostname) {
+    return ValidHostnameRegex.test(hostname);
+}
 /**
  * Resolve hostname to IP address,
  * returns null in case of error
  */
 module.exports = {
     resolve: function resolve(hostname) {
         var output,
-            nodeBinary = process.execPath,
-            scriptPath = path.join(__dirname, "../scripts/dns-lookup-script"),
+            nodeBinary = process.execPath;
+
+        if (!isValidHostName(hostname)) {
+            console.error('Invalid hostname:', hostname);
+            return null;
+        }
+
+        var scriptPath = path.join(__dirname, "../scripts/dns-lookup-script"),
             response,
             cmd = util.format('"%s" "%s" %s', nodeBinary, scriptPath, hostname);
 
         response = shell.exec(cmd, {silent: true});
         if (response && response.code === 0) {
diff --git a/package.json b/package.json
index 105bb6d..87ddc34 100644
--- a/package.json
+++ b/package.json
@@ -1,8 +1,8 @@
 {
   "name": "dns-sync",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "description": "dns-sync",
   "main": "index.js",
   "scripts": {
     "test": "make test"
   },
@@ -18,13 +18,13 @@
   ],
   "author": "Sanjeev Koranga",
   "license": "MIT",
   "readmeFilename": "README.md",
   "dependencies": {
-    "debug" : "~0.7",
-    "shelljs": "~0.2"
+    "debug" : "^2",
+    "shelljs": "~0.3"
   },
   "devDependencies": {
-    "mocha" : "~1",
-    "jshint" : "*"
+    "mocha" : "^1",
+    "jshint" : "^2"
   }
 }
diff --git a/test/test.js b/test/test.js
index 7a0a815..1bfd88b 100644
--- a/test/test.js
+++ b/test/test.js
@@ -14,6 +14,12 @@ describe('dns sync', function () {
     it('should fail to resolve dns', function () {
         assert.ok(!dnsSync.resolve('www.paypal.con'));
         assert.ok(!dnsSync.resolve('www.not-google.first'));
         assert.ok(!dnsSync.resolve('www.hello-yahoo.next'));
     });
+
+    it('should fail to resolve valid dns', function () {
+        assert.ok(!dnsSync.resolve("$(id > /tmp/foo)'"));
+        assert.ok(!dnsSync.resolve("cd /tmp; rm -f /tmp/echo; env 'x=() { (a)=>\' bash -c \"echo date\"; cat /tmp/echo"));
+        assert.ok(!dnsSync.resolve("$(grep -l -z '[^)]=() {' /proc/[1-9]*/environ | cut -d/ -f3)'"));
+    });
 });

Vulnerable code:             nodeBinary = process.execPath,
            scriptPath = path.join(__dirname, "../scripts/dns-lookup-script"),
  "version": "0.1.0",
    "debug" : "~0.7",
    "shelljs": "~0.2"
    "mocha" : "~1",
    "jshint" : "*"
Fixed code: //source - http://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
var ValidHostnameRegex = new RegExp("^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$");

function isValidHostName(hostname) {
    return ValidHostnameRegex.test(hostname);
}
            nodeBinary = process.execPath;

        if (!isValidHostName(hostname)) {
            console.error('Invalid hostname:', hostname);
            return null;
        }

        var scriptPath = path.join(__dirname, "../scripts/dns-lookup-script"),
  "version": "0.1.1",
    "debug" : "^2",
    "shelljs": "~0.3"
    "mocha" : "^1",
    "jshint" : "^2"

    it('should fail to resolve valid dns', function () {
        assert.ok(!dnsSync.resolve("$(id > /tmp/foo)'"));
        assert.ok(!dnsSync.resolve("cd /tmp; rm -f /tmp/echo; env 'x=() { (a)=>\' bash -c \"echo date\"; cat /tmp/echo"));
        assert.ok(!dnsSync.resolve("$(grep -l -z '[^)]=() {' /proc/[1-9]*/environ | cut -d/ -f3)'"));
    });