[feat] added tsr-detect-unsafe-properties-access rule; optimized imports in source files

webschik · webschik · commit fcc02dda34e6 · 2019-01-13T15:00:01.000+01:00
diff --git a/README.md b/README.md
@@ -31,6 +31,7 @@ By default `tslint-config-security` enables [all rules](#rules), but you may dis
     "tsr-detect-html-injection": false,
     "tsr-detect-unsafe-regexp": false
   }
+}
 ```
 
 
@@ -209,3 +210,34 @@ const myWindow = document.getElementById('myIFrame').contentWindow;
 
 myWindow.postMessage(message, "*"); // Noncompliant
 ```
+
+#### tsr-detect-unsafe-properties-access
+
+Detects a potential unsafe access to the object properties
+
+```js
+/* 
+
+It equals to `new Function(prop3)`
+
+const a = {};
+
+a["constructor"]["constructor"]("alert(1)")()
+ */
+ 
+// unsafe
+obj[prop1][prop2](prop3)
+
+// unsafe
+obj[prop1][prop2](prop3)()  
+ 
+```
+
+More information:
+* [Web Puzzlers - Securing Dynamic Systems](https://youtu.be/SkNWAjDRLDY)
+* [Defensive JavaScript](https://www.javascriptjanuary.com/blog/defensive-javascript)
+
+Solutions:
+* use [Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map)
+* use `.hasOwnProperty` check
+* use `Content-Security-Policy` on your page
diff --git a/index.js b/index.js
@@ -15,6 +15,7 @@ module.exports = {
         'tsr-disable-mustache-escape': [true],
         'tsr-detect-html-injection': [true],
         'tsr-detect-sql-literal-injection': [true],
-        'tsr-detect-unsafe-cross-origin-communication': [true]
+        'tsr-detect-unsafe-cross-origin-communication': [true],
+        'tsr-detect-unsafe-properties-access': [true]
     }
 };
diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "tslint-config-security",
-  "version": "1.14.0",
+  "version": "1.15.0",
   "description": "TSLint security rules",
   "main": "./index.js",
   "files": [
diff --git a/src/rules/tsrDetectBufferNoassertRule.ts b/src/rules/tsrDetectBufferNoassertRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 
 const readMethods: string[] = [
     'readUInt8',
diff --git a/src/rules/tsrDetectChildProcessRule.ts b/src/rules/tsrDetectChildProcessRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {StringLiteral, stringLiteralKinds} from '../node-kind';
 
 export class Rule extends Lint.Rules.AbstractRule {
diff --git a/src/rules/tsrDetectEvalWithExpressionRule.ts b/src/rules/tsrDetectEvalWithExpressionRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {stringLiteralKinds} from '../node-kind';
 import syntaxKindToName from '../syntax-kind-to-name';
 
diff --git a/src/rules/tsrDetectHtmlInjectionRule.ts b/src/rules/tsrDetectHtmlInjectionRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {stringLiteralKinds} from '../node-kind';
 
 export class Rule extends Lint.Rules.AbstractRule {
diff --git a/src/rules/tsrDetectNoCsrfBeforeMethodOverrideRule.ts b/src/rules/tsrDetectNoCsrfBeforeMethodOverrideRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 
 export class Rule extends Lint.Rules.AbstractRule {
     apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
diff --git a/src/rules/tsrDetectNonLiteralBufferRule.ts b/src/rules/tsrDetectNonLiteralBufferRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {stringLiteralKinds} from '../node-kind';
 
 export class Rule extends Lint.Rules.AbstractRule {
diff --git a/src/rules/tsrDetectNonLiteralRegexpRule.ts b/src/rules/tsrDetectNonLiteralRegexpRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {stringLiteralKinds} from '../node-kind';
 
 export class Rule extends Lint.Rules.AbstractRule {
diff --git a/src/rules/tsrDetectNonLiteralRequireRule.ts b/src/rules/tsrDetectNonLiteralRequireRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {stringLiteralKinds} from '../node-kind';
 
 export class Rule extends Lint.Rules.AbstractRule {
diff --git a/src/rules/tsrDetectPossibleTimingAttacksRule.ts b/src/rules/tsrDetectPossibleTimingAttacksRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {StringLiteral, stringLiteralKinds} from '../node-kind';
 
 const keywordMask = new RegExp(
diff --git a/src/rules/tsrDetectPseudoRandomBytesRule.ts b/src/rules/tsrDetectPseudoRandomBytesRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 
 export class Rule extends Lint.Rules.AbstractRule {
     apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
diff --git a/src/rules/tsrDetectSqlLiteralInjectionRule.ts b/src/rules/tsrDetectSqlLiteralInjectionRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {isSqlQuery} from '../is-sql-query';
 import {stringLiteralKinds} from '../node-kind';
 
diff --git a/src/rules/tsrDetectUnsafeCrossOriginCommunicationRule.ts b/src/rules/tsrDetectUnsafeCrossOriginCommunicationRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 
 export class Rule extends Lint.Rules.AbstractRule {
     apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
diff --git a/src/rules/tsrDetectUnsafePropertiesAccessRule.ts b/src/rules/tsrDetectUnsafePropertiesAccessRule.ts
@@ -0,0 +1,25 @@
+import * as Lint from 'tslint';
+import * as ts from 'typescript';
+
+export class Rule extends Lint.Rules.AbstractRule {
+    apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
+    }
+}
+
+class RuleWalker extends Lint.RuleWalker {
+    visitCallExpression(node: ts.CallExpression) {
+        const {expression, arguments: args} = node;
+
+        if (
+            expression &&
+            args &&
+            expression.kind === ts.SyntaxKind.ElementAccessExpression &&
+            args.find(ts.isIdentifier)
+        ) {
+            this.addFailureAtNode(node, 'Found unsafe properties access');
+        }
+
+        super.visitCallExpression(node);
+    }
+}
diff --git a/src/rules/tsrDetectUnsafeRegexpRule.ts b/src/rules/tsrDetectUnsafeRegexpRule.ts
@@ -1,9 +1,9 @@
-import * as ts from 'typescript';
+// @ts-ignore
+import * as isSafeRegexp from 'safe-regex';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import {StringLiteral, stringLiteralKinds} from '../node-kind';
 
-const isSafeRegexp = require('safe-regex');
-
 export class Rule extends Lint.Rules.AbstractRule {
     apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
diff --git a/src/rules/tsrDisableMustacheEscapeRule.ts b/src/rules/tsrDisableMustacheEscapeRule.ts
@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 
 export class Rule extends Lint.Rules.AbstractRule {
     apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
diff --git a/test/rules/tsr-detect-unsafe-properties-access/default/test.ts.lint b/test/rules/tsr-detect-unsafe-properties-access/default/test.ts.lint
@@ -0,0 +1,8 @@
+obj[prop1][prop2](prop3)
+~~~~~~~~~~~~~~~~~~~~~~~~    [Found unsafe properties access]
+
+obj[prop1][prop2](prop3)()
+~~~~~~~~~~~~~~~~~~~~~~~~    [Found unsafe properties access]
+
+obj[prop1][prop2].knownApiMethod()
+obj.prop1.prop2(prop3)
diff --git a/test/rules/tsr-detect-unsafe-properties-access/default/tslint.json b/test/rules/tsr-detect-unsafe-properties-access/default/tslint.json
@@ -0,0 +1,6 @@
+{
+  "rulesDirectory": "../../../../dist/rules",
+  "rules": {
+    "tsr-detect-unsafe-properties-access": true
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ module.exports = {`
`15`	`15`	`'tsr-disable-mustache-escape': [true],`
`16`	`16`	`'tsr-detect-html-injection': [true],`
`17`	`17`	`'tsr-detect-sql-literal-injection': [true],`
`18`		`- 'tsr-detect-unsafe-cross-origin-communication': [true]`
	`18`	`+ 'tsr-detect-unsafe-cross-origin-communication': [true],`
	`19`	`+ 'tsr-detect-unsafe-properties-access': [true]`
`19`	`20`	`}`
`20`	`21`	`};`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "tslint-config-security",`
`3`		`- "version": "1.14.0",`
	`3`	`+ "version": "1.15.0",`
`4`	`4`	`"description": "TSLint security rules",`
`5`	`5`	`"main": "./index.js",`
`6`	`6`	`"files": [`