[bug] fixed tsr-detect-non-literal-fs-filename rule (closes #5)

README.md

@@ -72,6 +72,29 @@ Detects variable in filename argument of `fs` calls, which might allow an attack
 
 More information: https://www.owasp.org/index.php/Path_Traversal
 
+**Known limitations**
+
+Due to the known issues in the typed TSLint rules
+
+* https://github.com/Microsoft/vscode-tslint/issues/70
+* https://github.com/Microsoft/vscode-tslint/blob/master/tslint/README.md#how-can-i-use-tslint-rules-that-require-type-information
+* https://github.com/Microsoft/vscode-tslint/issues/70
+
+ `tslint-config-security` module will analyze methods only on **fs** variable or on **'fs' module**. E.g.:
+
+```
+const fs = require('fs');
+
+fs.open(somePath); // triggers the error
+require('fs').symlink(path1, path2); // triggers the error
+require("fs").symlink(path1, path2); // triggers the error
+
+const myFs = require('fs');
+
+myFs.open(somePath); // no error
+```
+
+
 #### `tsr-detect-non-literal-regexp`
 
 Detects `RegExp(variable)`, which might allow an attacker to DOS your server with a long-running regular expression.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "tslint-config-security",
-  "version": "1.4.0",
+  "version": "1.5.0",
   "description": "TSLint security rules",
   "main": "./index.js",
   "scripts": {

src/rules/tsrDetectNonLiteralFsFilenameRule.ts

@@ -1,5 +1,5 @@
-import * as ts from 'typescript';
 import * as Lint from 'tslint';
+import * as ts from 'typescript';
 import fsModuleMethodsArgumentsInfo from '../fs-module-methods-arguments-info';
 
 export class Rule extends Lint.Rules.AbstractRule {
@@ -8,26 +8,33 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 }
 
+const expressionsToCheck: string[] = ['fs', `require('fs')`, `require("fs")`];
+
 class RuleWalker extends Lint.RuleWalker {
     visitPropertyAccessExpression (node: ts.PropertyAccessExpression) {
-        const name: ts.Identifier = node.name;
-        const methodName: string = name && name.text;
-        const parent: ts.CallExpression = node.parent as ts.CallExpression;
-        const fsArgsInfo: number[]|void = fsModuleMethodsArgumentsInfo.get(methodName);
-        const methodArguments: ts.NodeArray<ts.Expression> = parent && parent.arguments;
-
-        if (fsArgsInfo && methodArguments) {
-            const invalidArgumentIndices: number[] = fsArgsInfo.filter((index: number) => {
-                const arg: ts.Expression = methodArguments[index];
-
-                return Boolean(arg && arg.kind !== ts.SyntaxKind.StringLiteral);
-            });
-
-            if (invalidArgumentIndices[0] !== undefined) {
-                this.addFailureAtNode(
-                    node,
-                    `Found fs.${ methodName } with non-literal argument as index ${ invalidArgumentIndices.join(', ')}`
-                );
+        const {name, expression} = node;
+
+        if (name && node.parent && expression) {
+            const methodName: string = name.getText();
+            const parent: ts.CallExpression = node.parent as ts.CallExpression;
+            const fsArgsInfo: number[]|void = fsModuleMethodsArgumentsInfo.get(methodName);
+            const methodArguments: ts.NodeArray<ts.Expression> = parent.arguments;
+
+            if (fsArgsInfo && methodArguments && expressionsToCheck.includes(expression.getText())) {
+                const invalidArgumentIndices: number[] = fsArgsInfo.filter((index: number) => {
+                    const arg: ts.Expression = methodArguments[index];
+
+                    return Boolean(arg && arg.kind !== ts.SyntaxKind.StringLiteral);
+                });
+
+                if (invalidArgumentIndices[0] !== undefined) {
+                    const errorIndex: string = invalidArgumentIndices.join(', ');
+
+                    this.addFailureAtNode(
+                        node,
+                        `Found fs.${ methodName } with non-literal argument at index ${ errorIndex }`
+                    );
+                }
             }
         }
 

test/rules/tsr-detect-non-literal-fs-filename/default/test.ts.lint

@@ -1,12 +1,20 @@
+const fs = require('fs');
+
 const file1 = fs.open('test');
 const file2 = fs.open(test);
-              ~~~~~~~        [Found fs.open with non-literal argument as index 0]
+              ~~~~~~~        [Found fs.open with non-literal argument at index 0]
 
-fs.symlink(path1, path2);
-~~~~~~~~~~                [Found fs.symlink with non-literal argument as index 0, 1]
+require('fs').symlink(path1, path2);
+~~~~~~~~~~~~~~~~~~~~~                [Found fs.symlink with non-literal argument at index 0, 1]
 fs.symlink('path1', path2);
-~~~~~~~~~~                  [Found fs.symlink with non-literal argument as index 1]
-fs.symlink(path1, 'path2');
-~~~~~~~~~~                  [Found fs.symlink with non-literal argument as index 0]
+~~~~~~~~~~                  [Found fs.symlink with non-literal argument at index 1]
+require("fs").symlink(path1, 'path2');
+~~~~~~~~~~~~~~~~~~~~~                  [Found fs.symlink with non-literal argument at index 0]
 fs.symlink('path1', 'path2');
-fs.toString();
+fs.toString();
+
+const s = 'test';
+const o = {
+  open(a: string) {},
+};
+o.open(s);

tsconfig.json

@@ -1,6 +1,6 @@
 {
   "compilerOptions": {
-    "lib": ["es2015"],
+    "lib": ["es2018"],
     "target": "es5",
     "module": "commonjs",
     "moduleResolution": "node",
@@ -15,7 +15,7 @@
     "noImplicitReturns": true,
     "noImplicitThis": true,
     "pretty": true,
-    "removeComments": true
+    "removeComments": true,
   },
   "exclude": [
     "node_modules"