Merge pull request #3 from jesusprubio/master

Adam Baldwin · web-flow · commit d913b9e7a891 · 2017-02-09T09:45:25.000-08:00
A bit of love
diff --git a/.eslintrc b/.eslintrc
@@ -0,0 +1,3 @@
+{
+  "extends": "nodesecurity"
+}
diff --git a/README.md b/README.md
@@ -16,6 +16,7 @@ Add the following to your `.eslintrc` file:
   "security"
 ]
 ```
+
 ### Rules
 
 - `detect-unsafe-regex` - Locates potentially unsafe regular expressions
@@ -31,3 +32,18 @@ Add the following to your `.eslintrc` file:
 - `detect-possible-timing-attacks` - Detects insecure comparisons (== != !== ===)
 - `detect-pseudoRandomBytes` - Detects if pseudoRandomBytes() is in use
 
+
+## Developer guide
+
+- Use [GitHub pull requests](https://help.github.com/articles/using-pull-requests).
+- Conventions:
+ - We use our [custom ESLint setup](https://github.com/nodesecurity/eslint-config-nodesecurity).
+ - Please implement a test for each new rule and use this command to be sure the new code respects the style guide and the tests keep passing:
+ ```sh
+ npm run-script cont-int
+ ```
+
+### Tests
+```sh
+npm test
+```
diff --git a/package.json b/package.json
@@ -4,7 +4,9 @@
   "description": "Security rules for eslint",
   "main": "index.js",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "./node_modules/.bin/mocha test/**/*",
+    "lint": "./node_modules/.bin/eslint .",
+    "cont-int": "npm test && npm run-script lint"
   },
   "repository": {
     "type": "git",
@@ -25,6 +27,8 @@
     "safe-regex": "^1.1.0"
   },
   "devDependencies": {
-    "eslint": "^1.8.0"
+    "eslint": "^2.10.1",
+    "eslint-config-nodesecurity": "^1.3.1",
+    "mocha": "^2.4.5"
   }
 }
diff --git a/rules/detect-buffer-noassert.js b/rules/detect-buffer-noassert.js
@@ -63,7 +63,7 @@ module.exports = function(context) {
 
             if (index && node.parent && node.parent.arguments && node.parent.arguments[index] &&  node.parent.arguments[index].value) {
                 var token = context.getTokens(node)[0];
-                return context.report(node, 'found Buffer.' + node.property.name + ' with noAssert flag set true:\n\t' + getSource(token));
+                return context.report(node, 'Found Buffer.' + node.property.name + ' with noAssert flag set true:\n\t' + getSource(token));
                 
             }
         }
diff --git a/rules/detect-child-process.js b/rules/detect-child-process.js
@@ -28,15 +28,15 @@ module.exports = function(context) {
                     } else if (node.parent.type === 'AssignmentExpression' && node.parent.operator === '=') {
                         names.push(node.parent.left.name);
                     }
-                    return context.report(node, 'found require("child_process")\n\t' + getSource(token));
+                    return context.report(node, 'Found require("child_process")\n\t' + getSource(token));
                 }
             }
         },
         "MemberExpression": function (node) {
             var token = context.getTokens(node)[0];
             if (node.property.name === 'exec' && names.indexOf(node.object.name) > -1) {
                 if (node.parent && node.parent.arguments  && node.parent.arguments[0].type !== 'Literal') {
-                    return context.report(node, 'found child_process.exec() with non Literal first argument\n\t' + getSource(token));
+                    return context.report(node, 'Found child_process.exec() with non Literal first argument\n\t' + getSource(token));
                 }
             }
         }
diff --git a/rules/detect-non-literal-fs-filename.js b/rules/detect-non-literal-fs-filename.js
@@ -36,7 +36,7 @@ module.exports = function(context) {
 
             if (result.length > 0) {
                 var token = context.getTokens(node)[0];
-                return context.report(node, 'found fs.' + node.property.name + ' with non literal argument at index ' + result.join(',') + '\n\t' + getSource(token));
+                return context.report(node, 'Found fs.' + node.property.name + ' with non literal argument at index ' + result.join(',') + '\n\t' + getSource(token));
             }
 
 
diff --git a/rules/detect-non-literal-regexp.js b/rules/detect-non-literal-regexp.js
@@ -16,12 +16,12 @@ module.exports = function(context) {
                 return token.loc.start.line + ':  ' + context.getSourceLines().slice(token.loc.start.line - 1, token.loc.end.line).join('\n\t');
         }
         return {
-                "CallExpression": function(node) {
+                "NewExpression": function(node) {
                         if (node.callee.name === 'RegExp') {
                                 var args = node.arguments;
                                 if (args && args.length > 0 && args[0].type !== 'Literal') {
                                         var token = context.getTokens(node)[0];
-                                        return context.report(node, 'found non-literal argument to RegExp Constructor\n\t' + getSource(token));
+                                        return context.report(node, 'Found non-literal argument to RegExp Constructor\n\t' + getSource(token));
                                 }
                         }
 
diff --git a/rules/detect-non-literal-require.js b/rules/detect-non-literal-require.js
@@ -21,7 +21,7 @@ module.exports = function(context) {
                 var args = node.arguments;
                 if (args && args.length > 0 && args[0].type !== 'Literal') {
                     var token = context.getTokens(node)[0];
-                    return context.report(node, 'found non-literal argument in require\n\t' + getSource(token));
+                    return context.report(node, 'Found non-literal argument in require\n\t' + getSource(token));
                 }
             }
 
diff --git a/rules/detect-pseudoRandomBytes.js b/rules/detect-pseudoRandomBytes.js
@@ -19,7 +19,7 @@ module.exports = function(context) {
         "MemberExpression": function (node) {
             if (node.property.name === 'pseudoRandomBytes') {
                 var token = context.getTokens(node)[0];
-                return context.report(node, 'found crypto.pseudoRandomBytes which does not produce cryptographically strong numbers:\n\t' + getSource(token));
+                return context.report(node, 'Found crypto.pseudoRandomBytes which does not produce cryptographically strong numbers:\n\t' + getSource(token));
             }
         }
 
diff --git a/test/detect-buffer-noassert.js b/test/detect-buffer-noassert.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-buffer-noassert';
+const Rule = require(`../rules/${ruleName}`);
+
+const invalid = 'a.readUInt8(0, true);';
+
+
+tester.run(ruleName, Rule, {
+  valid: [{ code: 'a.readUInt8(0);' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found Buffer.readUInt8 with noAssert flag set true:\n\t1:  ${invalid}` }]
+    }
+  ]
+});
+
+tester.run(`${ruleName} (false)`, Rule, {
+  valid: [{ code: 'a.readUInt8(0, false);' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found Buffer.readUInt8 with noAssert flag set true:\n\t1:  ${invalid}` }]
+    }
+  ]
+});
diff --git a/test/detect-child-process.js b/test/detect-child-process.js
@@ -0,0 +1,35 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-child-process';
+const Rule = require(`../rules/${ruleName}`);
+
+const valid = 'child_process.exec(\'ls\')';
+const invalidRequire = 'require(\'child_process\')';
+const invalidExec = 'var child = require(\'child_process\'); child.exec(com)';
+
+
+tester.run(`${ruleName} (require("child_process"))`, Rule, {
+  valid: [{ code: valid }],
+  invalid: [
+    {
+      code: invalidRequire,
+      errors: [{ message: `Found require("child_process")\n\t1:  ${invalidRequire}` }]
+    }
+  ]
+});
+
+
+tester.run(`${ruleName} (child_process.exec() wih non literal 1st arg.)`, Rule, {
+  valid: [{ code: valid }],
+  invalid: [
+    {
+      code: invalidExec,
+      errors: [
+        { message: `Found require("child_process")\n\t1:  ${invalidExec}` },
+        { message: `Found child_process.exec() with non Literal first argument\n\t1:  ${invalidExec}` }]
+    }
+  ]
+});
diff --git a/test/detect-disable-mustache-escape.js b/test/detect-disable-mustache-escape.js
@@ -0,0 +1,17 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-disable-mustache-escape';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'escapeMarkup = false' }],
+  invalid: [
+    {
+      code: 'a.escapeMarkup = false',
+      errors: [{ message: 'Markup escaping disabled.' }]
+    }
+  ]
+});
diff --git a/test/detect-eval-with-expression.js b/test/detect-eval-with-expression.js
@@ -0,0 +1,17 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-eval-with-expression';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'eval(\'alert()\')' }],
+  invalid: [
+    {
+      code: 'eval(a);',
+      errors: [{ message: 'eval with argument of type Identifier' }]
+    }
+  ]
+});
diff --git a/test/detect-new-buffer.js b/test/detect-new-buffer.js
@@ -0,0 +1,18 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-new-buffer';
+const invalid = 'var a = new Buffer(c)';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'var a = new Buffer(\'test\')' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found new Buffer\n\t1:  ${invalid}` }]
+    }
+  ]
+});
diff --git a/test/detect-no-csrf-before-method-override.js b/test/detect-no-csrf-before-method-override.js
@@ -0,0 +1,17 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-no-csrf-before-method-override';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'express.methodOverride();express.csrf()' }],
+  invalid: [
+    {
+      code: 'express.csrf();express.methodOverride()',
+      errors: [{ message: 'express.csrf() middleware found before express.methodOverride()' }]
+    }
+  ]
+});
diff --git a/test/detect-non-literal-fs-filename.js b/test/detect-non-literal-fs-filename.js
@@ -0,0 +1,19 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const invalid = 'var a = fs.open(c)';
+
+const ruleName = 'detect-non-literal-fs-filename';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'var a = fs.open(\'test\')' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found fs.open with non literal argument at index 0\n\t1:  ${invalid}` }]
+    }
+  ]
+});
diff --git a/test/detect-non-literal-regexp.js b/test/detect-non-literal-regexp.js
@@ -0,0 +1,18 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-non-literal-regexp';
+const invalid = 'var a = new RegExp(c, \'i\')';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'var a = new RegExp(\'ab+c\', \'i\')' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found non-literal argument to RegExp Constructor\n\t1:  ${invalid}` }]
+    }
+  ]
+});
diff --git a/test/detect-non-literal-require.js b/test/detect-non-literal-require.js
@@ -0,0 +1,18 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-non-literal-require';
+const invalid = 'var a = require(c)';
+
+
+tester.run(ruleName, require(`../rules/${ruleName}`), {
+  valid: [{ code: 'var a = require(\'b\')' }],
+  invalid: [
+    {
+      code: invalid,
+      errors: [{ message: `Found non-literal argument in require\n\t1:  ${invalid}` }]
+    }
+  ]
+});
diff --git a/test/detect-object-injection.js b/test/detect-object-injection.js
@@ -0,0 +1,47 @@
+'use strict';
+
+const RuleTester = require('eslint').RuleTester;
+const tester = new RuleTester();
+
+const ruleName = 'detect-object-injection';
+
+const Rule = require(`../rules/${ruleName}`);
+
+const valid = 'var a = {};';
+// const invalidVariable = "TODO";
+// const invalidFunction = "TODO";
+const invalidGeneric = 'var a = {}; a[b] = 4';
+
+
+// TODO
+// tester.run(`${ruleName} (Variable Assigned to)`, Rule, {
+//   valid: [{ code: valid }],
+//   invalid: [
+//     {
+//       code: invalidVariable,
+//       errors: [{ message: `Variable Assigned to Object Injection Sink: <input>: 1\n\t${invalidVariable}\n\n` }]
+//     }
+//   ]
+// });
+//
+//
+// tester.run(`${ruleName} (Function)`, Rule, {
+//   valid: [{ code: valid }],
+//   invalid: [
+//     {
+//       code: invalidFunction,
+//       errors: [{ message: `Variable Assigned to Object Injection Sink: <input>: 1\n\t${invalidFunction}\n\n` }]
+//     }
+//   ]
+// });
+
+
+tester.run(`${ruleName} (Generic)`, Rule, {
+  valid: [{ code: valid }],
+  invalid: [
+    {
+      code: invalidGeneric,
+      errors: [{ message: `Generic Object Injection Sink: <input>: 1\n\t${invalidGeneric}\n\n` }]
+    }
+  ]
+});
diff --git a/test/detect-possible-timing-attacks.js b/test/detect-possible-timing-attacks.js
diff --git a/test/detect-pseudoRandomBytes.js b/test/detect-pseudoRandomBytes.js
diff --git a/test/detect-unsafe-regexp.js b/test/detect-unsafe-regexp.js

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+{`
	`2`	`+ "extends": "nodesecurity"`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ module.exports = function(context) {`
`63`	`63`
`64`	`64`	`if (index && node.parent && node.parent.arguments && node.parent.arguments[index] && node.parent.arguments[index].value) {`
`65`	`65`	`var token = context.getTokens(node)[0];`
`66`		`- return context.report(node, 'found Buffer.' + node.property.name + ' with noAssert flag set true:\n\t' + getSource(token));`
	`66`	`+ return context.report(node, 'Found Buffer.' + node.property.name + ' with noAssert flag set true:\n\t' + getSource(token));`
`67`	`67`
`68`	`68`	`}`
`69`	`69`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,15 +28,15 @@ module.exports = function(context) {`
`28`	`28`	`} else if (node.parent.type === 'AssignmentExpression' && node.parent.operator === '=') {`
`29`	`29`	`names.push(node.parent.left.name);`
`30`	`30`	`}`
`31`		`- return context.report(node, 'found require("child_process")\n\t' + getSource(token));`
	`31`	`+ return context.report(node, 'Found require("child_process")\n\t' + getSource(token));`
`32`	`32`	`}`
`33`	`33`	`}`
`34`	`34`	`},`
`35`	`35`	`"MemberExpression": function (node) {`
`36`	`36`	`var token = context.getTokens(node)[0];`
`37`	`37`	`if (node.property.name === 'exec' && names.indexOf(node.object.name) > -1) {`
`38`	`38`	`if (node.parent && node.parent.arguments && node.parent.arguments[0].type !== 'Literal') {`
`39`		`- return context.report(node, 'found child_process.exec() with non Literal first argument\n\t' + getSource(token));`
	`39`	`+ return context.report(node, 'Found child_process.exec() with non Literal first argument\n\t' + getSource(token));`
`40`	`40`	`}`
`41`	`41`	`}`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ module.exports = function(context) {`
`36`	`36`
`37`	`37`	`if (result.length > 0) {`
`38`	`38`	`var token = context.getTokens(node)[0];`
`39`		`- return context.report(node, 'found fs.' + node.property.name + ' with non literal argument at index ' + result.join(',') + '\n\t' + getSource(token));`
	`39`	`+ return context.report(node, 'Found fs.' + node.property.name + ' with non literal argument at index ' + result.join(',') + '\n\t' + getSource(token));`
`40`	`40`	`}`
`41`	`41`
`42`	`42`
Original file line number	Diff line number	Diff line change
`@@ -16,12 +16,12 @@ module.exports = function(context) {`
`16`	`16`	`return token.loc.start.line + ': ' + context.getSourceLines().slice(token.loc.start.line - 1, token.loc.end.line).join('\n\t');`
`17`	`17`	`}`
`18`	`18`	`return {`
`19`		`- "CallExpression": function(node) {`
	`19`	`+ "NewExpression": function(node) {`
`20`	`20`	`if (node.callee.name === 'RegExp') {`
`21`	`21`	`var args = node.arguments;`
`22`	`22`	`if (args && args.length > 0 && args[0].type !== 'Literal') {`
`23`	`23`	`var token = context.getTokens(node)[0];`
`24`		`- return context.report(node, 'found non-literal argument to RegExp Constructor\n\t' + getSource(token));`
	`24`	`+ return context.report(node, 'Found non-literal argument to RegExp Constructor\n\t' + getSource(token));`
`25`	`25`	`}`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ module.exports = function(context) {`
`21`	`21`	`var args = node.arguments;`
`22`	`22`	`if (args && args.length > 0 && args[0].type !== 'Literal') {`
`23`	`23`	`var token = context.getTokens(node)[0];`
`24`		`- return context.report(node, 'found non-literal argument in require\n\t' + getSource(token));`
	`24`	`+ return context.report(node, 'Found non-literal argument in require\n\t' + getSource(token));`
`25`	`25`	`}`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ module.exports = function(context) {`
`19`	`19`	`"MemberExpression": function (node) {`
`20`	`20`	`if (node.property.name === 'pseudoRandomBytes') {`
`21`	`21`	`var token = context.getTokens(node)[0];`
`22`		`- return context.report(node, 'found crypto.pseudoRandomBytes which does not produce cryptographically strong numbers:\n\t' + getSource(token));`
	`22`	`+ return context.report(node, 'Found crypto.pseudoRandomBytes which does not produce cryptographically strong numbers:\n\t' + getSource(token));`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`