Skip to content

A bit of love #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 9, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "nodesecurity"
}
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -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
```
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
2 changes: 1 addition & 1 deletion rules/detect-buffer-noassert.js
Original file line number Diff line number Diff line change
@@ -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));

}
}
4 changes: 2 additions & 2 deletions rules/detect-child-process.js
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
2 changes: 1 addition & 1 deletion rules/detect-non-literal-fs-filename.js
Original file line number Diff line number Diff line change
@@ -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));
}


4 changes: 2 additions & 2 deletions rules/detect-non-literal-regexp.js
Original file line number Diff line number Diff line change
@@ -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));
}
}

2 changes: 1 addition & 1 deletion rules/detect-non-literal-require.js
Original file line number Diff line number Diff line change
@@ -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));
}
}

2 changes: 1 addition & 1 deletion rules/detect-pseudoRandomBytes.js
Original file line number Diff line number Diff line change
@@ -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));
}
}

30 changes: 30 additions & 0 deletions test/detect-buffer-noassert.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
35 changes: 35 additions & 0 deletions test/detect-child-process.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
17 changes: 17 additions & 0 deletions test/detect-disable-mustache-escape.js
Original file line number Diff line number Diff line change
@@ -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.' }]
}
]
});
17 changes: 17 additions & 0 deletions test/detect-eval-with-expression.js
Original file line number Diff line number Diff line change
@@ -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' }]
}
]
});
18 changes: 18 additions & 0 deletions test/detect-new-buffer.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
17 changes: 17 additions & 0 deletions test/detect-no-csrf-before-method-override.js
Original file line number Diff line number Diff line change
@@ -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()' }]
}
]
});
19 changes: 19 additions & 0 deletions test/detect-non-literal-fs-filename.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
18 changes: 18 additions & 0 deletions test/detect-non-literal-regexp.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
18 changes: 18 additions & 0 deletions test/detect-non-literal-require.js
Original file line number Diff line number Diff line change
@@ -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}` }]
}
]
});
47 changes: 47 additions & 0 deletions test/detect-object-injection.js
Original file line number Diff line number Diff line change
@@ -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` }]
}
]
});
36 changes: 36 additions & 0 deletions test/detect-possible-timing-attacks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const tester = new RuleTester();

const ruleName = 'detect-possible-timing-attacks';
const Rule = require(`../rules/${ruleName}`);

const valid = 'if (age === 5) {}';
const invalidLeft = 'if (password === \'mypass\') {}';
const invalidRigth = 'if (\'mypass\' === password) {}';


// We only check with one string "password" and operator "==="
// to KISS.

tester.run(`${ruleName} (left side)`, Rule, {
valid: [{ code: valid }],
invalid: [
{
code: invalidLeft,
errors: [{ message: `Potential timing attack, left side: true\n\t1: ${invalidLeft}` }]
}
]
});


tester.run(`${ruleName} (right side)`, Rule, {
valid: [{ code: valid }],
invalid: [
{
code: invalidRigth,
errors: [{ message: `Potential timing attack, right side: true\n\t1: ${invalidRigth}` }]
}
]
});
18 changes: 18 additions & 0 deletions test/detect-pseudoRandomBytes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const tester = new RuleTester();

const ruleName = 'detect-pseudoRandomBytes';
const invalid = 'crypto.pseudoRandomBytes';


tester.run(ruleName, require(`../rules/${ruleName}`), {
valid: [{ code: 'crypto.randomBytes' }],
invalid: [
{
code: invalid,
errors: [{ message: `Found crypto.pseudoRandomBytes which does not produce cryptographically strong numbers:\n\t1: ${invalid}` }]
}
]
});
29 changes: 29 additions & 0 deletions test/detect-unsafe-regexp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const tester = new RuleTester();

const ruleName = 'detect-unsafe-regex';
const Rule = require(`../rules/${ruleName}`);


tester.run(ruleName, Rule, {
valid: [{ code: '/^\d+1337\d+$/' }],
invalid: [
{
code: '/(x+x+)+y/',
errors: [{ message: 'Unsafe Regular Expression' }]
}
]
});


tester.run(`${ruleName} (new RegExp)`, Rule, {
valid: [{ code: 'new RegExp(\'^\d+1337\d+$\')' }],
invalid: [
{
code: 'new RegExp(\'x+x+)+y\')',
errors: [{ message: 'Unsafe Regular Expression (new RegExp)' }]
}
]
});