[#255] new security rule: detect-non-literal-require

closes #255

README.md

@@ -130,6 +130,7 @@ Rule Name   | Description | Since
 `no-unused-imports`             | Deprecated - This rule is now covered by TSLint's no-unused-variables rule. However, it can still be useful to enable this rule and pair it with the fix-no-unused-imports formatter. | 0.0.1
 `no-var-self`                   | Do not use `var self = this`; instead, manage scope with arrow functions/lambdas. Self variables are a common practice in JavaScript but can be avoided in TypeScript. By default the rule bans any assignments of the `this` reference. If you want to enforce a naming convention or allow some usages then configure the rule with a regex. By default the rule is configured with `(?!)` which matches nothing. You can pass `^self$` to allow variables named self or pass `^(?!self$)` to allow anything other than self, for example| 2.0.8
 `no-with-statement`             | Do not use with statements. Assign the item to a new variable instead | 0.0.1
+`non-literal-require`           | Detect `require()` function calls for something that is not a string literal. For security reasons, it is best to only require() string literals. Otherwise, it is perhaps possible for an attacker to somehow change the value and download arbitrary Javascript into your page. | 2.0.14
 `possible-timing-attack`        | Avoid timing attacks by not making direct string comparisons to sensitive data. Do not compare against variables named password, secret, api, apiKey, token, auth, pass, or hash. For more info see [Using Node.js Event Loop for Timing Attacks](https://snyk.io/blog/node-js-timing-attack-ccc-ctf/) | 2.0.11
 `prefer-array-literal`          | Use array literal syntax when declaring or instantiating array types. For example, prefer the Javascript form of string[] to the TypeScript form Array<string>. Prefer '[]' to 'new Array()'. Prefer '[4, 5]' to 'new Array(4, 5)'. Prefer '[undefined, undefined]' to 'new Array(4)'. Since 2.0.10, this rule can be configured to allow Array type parameters. To ignore type parameters, configure the rule with the values: `[ true, { 'allow-type-parameters': true } ]`| 1.0, 2.0.10
 `prefer-const`                  | Use `const` to declare variables if they are only assigned a value once. | 2.0.6

src/nonLiteralRequireRule.ts

@@ -0,0 +1,63 @@
+import * as ts from 'typescript';
+import * as Lint from 'tslint/lib/lint';
+
+import {ErrorTolerantWalker} from './utils/ErrorTolerantWalker';
+import {ExtendedMetadata} from './utils/ExtendedMetadata';
+import {SyntaxKind} from './utils/SyntaxKind';
+import {AstUtils} from './utils/AstUtils';
+import {Utils} from './utils/Utils';
+
+const FAILURE_STRING: string = 'Non-literal (insecure) parameter passed to require(): ';
+
+/**
+ * Implementation of the non-literal-require rule.
+ */
+export class Rule extends Lint.Rules.AbstractRule {
+
+    public static metadata: ExtendedMetadata = {
+        ruleName: 'non-literal-require',
+        type: 'functionality',
+        description: 'Detect require includes that are not for string literals',
+        options: null,
+        issueClass: 'SDL',
+        issueType: 'Error',
+        severity: 'Critical',
+        level: 'Mandatory',
+        group: 'Security',
+        commonWeaknessEnumeration: '95,676'
+    };
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new NonLiteralRequireRuleWalker(sourceFile, this.getOptions()));
+    }
+}
+
+class NonLiteralRequireRuleWalker extends ErrorTolerantWalker {
+
+    protected visitCallExpression(node: ts.CallExpression): void {
+        if (AstUtils.getFunctionName(node) === 'require'
+            && AstUtils.getFunctionTarget(node) == null
+            && node.arguments.length > 0) {
+
+            if (node.arguments[0].kind === SyntaxKind.current().ArrayLiteralExpression) {
+                const arrayExp: ts.ArrayLiteralExpression = <ts.ArrayLiteralExpression>node.arguments[0];
+                arrayExp.elements.forEach((initExpression: ts.Expression): void => {
+                    if (initExpression.kind !== SyntaxKind.current().StringLiteral) {
+                        this.fail(initExpression);
+                    }
+                });
+            } else if (node.arguments[0].kind !== SyntaxKind.current().StringLiteral) {
+                this.fail(node.arguments[0]);
+            }
+        }
+        super.visitCallExpression(node);
+    }
+
+    private fail(expression: ts.Expression): void {
+        const start: number = expression.getStart();
+        const width: number  = expression.getWidth();
+        const message: string  = FAILURE_STRING + Utils.trimTo(expression.getText(), 25);
+        this.addFailure(this.createFailure(start, width, message));
+
+    }
+}

src/tests/NonLiteralRequireRuleTests.ts

@@ -0,0 +1,83 @@
+import {TestHelper} from './TestHelper';
+
+/**
+ * Unit tests.
+ */
+describe('nonLiteralRequireRule', () : void => {
+
+    const ruleName : string = 'non-literal-require';
+
+    it('should pass on imports', () : void => {
+        const script : string = `
+            import React = require('react');
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [ ]);
+    });
+
+    it('should pass on string literals', () : void => {
+        const script : string = `
+            const myModule = require('myModule');
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [ ]);
+    });
+
+    it('should pass on empty array', () : void => {
+        const script : string = `
+            const myModule = require([]);
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [ ]);
+    });
+
+    it('should pass on array of strings', () : void => {
+        const script : string = `
+            const myModule = require(['myModule']);
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [ ]);
+    });
+
+    it('should fail on non string literal', () : void => {
+        const script : string = `
+            const moduleName = 'myModule';
+            const myModule = require(moduleName);`;
+
+        TestHelper.assertViolations(ruleName, script, [
+            {
+                "failure": "Non-literal (insecure) parameter passed to require(): moduleName",
+                "name": "file.ts",
+                "ruleName": "non-literal-require",
+                "startPosition": { "character": 38, "line": 3 }
+            }
+        ]);
+    });
+
+    it('should fail on non-string array element', () : void => {
+        const script : string = `
+            let myModule = require([
+                'myModule',
+                somethingElse,
+                'otherModule',
+                getModuleName()
+            ]);
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [
+            {
+                "failure": "Non-literal (insecure) parameter passed to require(): somethingElse",
+                "name": "file.ts",
+                "ruleName": "non-literal-require",
+                "startPosition": { "character": 17, "line": 4 }
+            },
+            {
+                "failure": "Non-literal (insecure) parameter passed to require(): getModuleName()",
+                "name": "file.ts",
+                "ruleName": "non-literal-require",
+                "startPosition": { "character": 17, "line": 6 }
+            }
+        ]);
+    });
+
+});

test-data/references.ts

@@ -1,2 +1,3 @@
 /// <reference path="../typings/react.d.ts" />
 /// <reference path="../typings/underscore.d.ts" />
+/// <reference path="../typings/require.d.ts" />

tslint.json

@@ -23,7 +23,10 @@
     "forin": true,
     "function-name": true,
     "import-name": true,
-    "indent": [true, "spaces"],
+    "indent": [
+      true,
+      "spaces"
+    ],
     "interface-name": false,
     "jquery-deferred-must-complete": true,
     "jsdoc-format": false,
@@ -218,6 +221,7 @@
     "react-a11y-anchors": true,
     "react-anchor-blank-noopener": true,
     "insecure-random": true,
-    "possible-timing-attack": true
+    "possible-timing-attack": true,
+    "non-literal-require": true
   }
-}
+}