Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
[refactor] rebuilt AST walkers to improve performance https://palanti…
Browse files Browse the repository at this point in the history
  • Loading branch information
webschik committed Apr 13, 2019
1 parent 8d6551f commit 1186f3f
Show file tree
Hide file tree
Showing 18 changed files with 426 additions and 326 deletions.
2 changes: 1 addition & 1 deletion npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tslint-config-security",
"version": "1.15.0",
"version": "1.16.0",
"description": "TSLint security rules",
"main": "./index.js",
"files": [
Expand Down
46 changes: 25 additions & 21 deletions src/rules/tsrDetectBufferNoassertRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,39 @@ const writeMethods: string[] = [

export class Rule extends Lint.Rules.AbstractRule {
apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
return this.applyWithFunction(sourceFile, walk);
}
}

class RuleWalker extends Lint.RuleWalker {
visitPropertyAccessExpression(node: ts.PropertyAccessExpression) {
const {name} = node;
const parent: ts.CallExpression = node.parent as ts.CallExpression;
function walk(ctx: Lint.WalkContext<void>) {
function visitNode(node: ts.Node): void {
if (node.kind === ts.SyntaxKind.PropertyAccessExpression) {
const {name, expression} = node as ts.PropertyAccessExpression;
const parent: ts.CallExpression = node.parent as ts.CallExpression;

if (parent && parent.kind === ts.SyntaxKind.CallExpression && node.expression && name) {
const methodName: string = name.getText();
let argumentIndex: number = -1;
if (parent && parent.kind === ts.SyntaxKind.CallExpression && expression && name) {
const methodName: string = name.text;
let argumentIndex: number = -1;

if (readMethods.indexOf(methodName) !== -1) {
argumentIndex = 1;
} else if (writeMethods.indexOf(methodName) !== -1) {
argumentIndex = 2;
}
if (readMethods.indexOf(methodName) !== -1) {
argumentIndex = 1;
} else if (writeMethods.indexOf(methodName) !== -1) {
argumentIndex = 2;
}

if (
argumentIndex !== -1 &&
parent.arguments &&
parent.arguments[argumentIndex] &&
parent.arguments[argumentIndex].kind === ts.SyntaxKind.TrueKeyword
) {
this.addFailureAtNode(node, `Found Buffer.${methodName} with noAssert flag set true`);
if (
argumentIndex !== -1 &&
parent.arguments &&
parent.arguments[argumentIndex] &&
parent.arguments[argumentIndex].kind === ts.SyntaxKind.TrueKeyword
) {
ctx.addFailureAtNode(node, `Found Buffer.${methodName} with noAssert flag set true`);
}
}
}

super.visitPropertyAccessExpression(node);
return ts.forEachChild(node, visitNode);
}

return ts.forEachChild(ctx.sourceFile, visitNode);
}
80 changes: 46 additions & 34 deletions src/rules/tsrDetectChildProcessRule.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,60 @@
import * as Lint from 'tslint';
import * as ts from 'typescript';
import {StringLiteral, stringLiteralKinds} from '../node-kind';
import {stringLiteralKinds} from '../node-kind';

export class Rule extends Lint.Rules.AbstractRule {
apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
return this.applyWithFunction(sourceFile, walk);
}
}

class RuleWalker extends Lint.RuleWalker {
private names: string[] = [];

visitCallExpression(node: ts.CallExpression) {
const {expression} = node;
const firstArgument: StringLiteral = node.arguments && (node.arguments[0] as StringLiteral);

if (
firstArgument &&
expression &&
stringLiteralKinds.includes(firstArgument.kind) &&
firstArgument.text === 'child_process' &&
expression.getText() === 'require'
) {
const parent: ts.VariableDeclaration = node.parent as ts.VariableDeclaration;

this.names.length = 0;

if (parent && parent.kind === ts.SyntaxKind.VariableDeclaration) {
this.names.push(parent.name.getText());
function walk(ctx: Lint.WalkContext<void>) {
const names: string[] = [];

function visitNode(node: ts.Node): void {
switch (node.kind) {
case ts.SyntaxKind.CallExpression: {
const {expression, arguments: args} = node as ts.CallExpression;
const firstArgument = args && args[0];

if (
firstArgument &&
expression &&
stringLiteralKinds.includes(firstArgument.kind) &&
(firstArgument as ts.StringLiteral).text === 'child_process' &&
(expression as ts.StringLiteral).text === 'require'
) {
const parent: ts.VariableDeclaration = node.parent as ts.VariableDeclaration;

names.length = 0;

if (parent && parent.kind === ts.SyntaxKind.VariableDeclaration) {
names.push((parent.name as ts.Identifier).text);
}

ctx.addFailureAtNode(node, 'Found require("child_process")');
}
break;
}

this.addFailureAtNode(node, 'Found require("child_process")');
case ts.SyntaxKind.PropertyAccessExpression: {
const {name, expression} = node as ts.PropertyAccessExpression;

if (
name &&
expression &&
name.text === 'exec' &&
names.indexOf((expression as ts.Identifier).text) >= 0
) {
ctx.addFailureAtNode(node, 'Found child_process.exec() with non StringLiteral first argument');
}
break;
}
default:
//
}

super.visitCallExpression(node);
return ts.forEachChild(node, visitNode);
}

visitPropertyAccessExpression(node: ts.PropertyAccessExpression) {
const {name, expression} = node;

if (name && expression && name.getText() === 'exec' && this.names.indexOf(expression.getText()) >= 0) {
this.addFailureAtNode(node, 'Found child_process.exec() with non StringLiteral first argument');
}

super.visitPropertyAccessExpression(node);
}
return ts.forEachChild(ctx.sourceFile, visitNode);
}
50 changes: 32 additions & 18 deletions src/rules/tsrDetectEvalWithExpressionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,44 @@ import syntaxKindToName from '../syntax-kind-to-name';

export class Rule extends Lint.Rules.AbstractRule {
apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
return this.applyWithFunction(sourceFile, walk);
}
}

class RuleWalker extends Lint.RuleWalker {
visitCallExpression(node: ts.CallExpression) {
const firstArgument: ts.Expression = node.arguments[0];
function walk(ctx: Lint.WalkContext<void>) {
function visitNode(node: ts.Node): void {
switch (node.kind) {
case ts.SyntaxKind.CallExpression: {
const {expression, arguments: args} = node as ts.CallExpression;
const firstArgument: ts.Expression | undefined = args && args[0];

if (node.expression.getText() === 'eval' && firstArgument && !stringLiteralKinds.includes(firstArgument.kind)) {
this.addFailureAtNode(node, `eval with argument of type ${syntaxKindToName(firstArgument.kind)}`);
}

super.visitCallExpression(node);
}
if (
firstArgument &&
(expression as ts.StringLiteral).text === 'eval' &&
!stringLiteralKinds.includes(firstArgument.kind)
) {
ctx.addFailureAtNode(node, `eval with argument of type ${syntaxKindToName(firstArgument.kind)}`);
}
break;
}
case ts.SyntaxKind.NewExpression: {
const {expression, arguments: args} = node as ts.NewExpression;

visitNewExpression(node: ts.NewExpression) {
if (
node.arguments &&
node.expression.getText() === 'Function' &&
node.arguments.some((node: ts.Node) => !stringLiteralKinds.includes(node.kind))
) {
this.addFailureAtNode(node, 'Found function constructor with non-literal argument');
if (
args &&
(expression as ts.StringLiteral).text === 'Function' &&
args.some((node: ts.Node) => !stringLiteralKinds.includes(node.kind))
) {
ctx.addFailureAtNode(node, 'Found function constructor with non-literal argument');
}
break;
}
default:
//
}

super.visitNewExpression(node);
return ts.forEachChild(node, visitNode);
}

return ts.forEachChild(ctx.sourceFile, visitNode);
}
75 changes: 43 additions & 32 deletions src/rules/tsrDetectHtmlInjectionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,51 +4,62 @@ import {stringLiteralKinds} from '../node-kind';

export class Rule extends Lint.Rules.AbstractRule {
apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
return this.applyWithFunction(sourceFile, walk);
}
}

const unsafeDocumentHtmlMethods: string[] = ['writeln', 'write'];
const unsafeElementHtmlMethods: string[] = ['insertAdjacentHTML'];
const unsafeElementHtmlProps: string[] = ['outerHTML', 'innerHTML'];

class RuleWalker extends Lint.RuleWalker {
visitPropertyAccessExpression(node: ts.PropertyAccessExpression) {
const expression: ts.Identifier = node.expression as ts.Identifier;
const name: ts.Identifier = node.name;
const parent: ts.CallExpression = node.parent as ts.CallExpression;
const firstArgument: undefined | ts.Expression = parent && parent.arguments && parent.arguments[0];
function walk(ctx: Lint.WalkContext<void>) {
function visitNode(node: ts.Node): void {
switch (node.kind) {
case ts.SyntaxKind.PropertyAccessExpression: {
const {expression, name} = node as ts.PropertyAccessExpression;
const parent: ts.CallExpression = node.parent as ts.CallExpression;
const firstArgument: undefined | ts.Expression = parent && parent.arguments && parent.arguments[0];

if (expression && name && firstArgument && !stringLiteralKinds.includes(firstArgument.kind)) {
const method: string = name.text;
if (expression && name && firstArgument && !stringLiteralKinds.includes(firstArgument.kind)) {
const method: string = name.text;

if (expression.text === 'document' && unsafeDocumentHtmlMethods.includes(method)) {
this.addFailureAtNode(parent, `Found document.${method} with non-literal argument`);
} else if (unsafeElementHtmlMethods.includes(method)) {
this.addFailureAtNode(parent, `Found Element.${method} with non-literal argument`);
if (
(expression as ts.Identifier).text === 'document' &&
unsafeDocumentHtmlMethods.includes(method)
) {
ctx.addFailureAtNode(parent, `Found document.${method} with non-literal argument`);
} else if (unsafeElementHtmlMethods.includes(method)) {
ctx.addFailureAtNode(parent, `Found Element.${method} with non-literal argument`);
}
}

break;
}
}
case ts.SyntaxKind.BinaryExpression: {
const {left, right, operatorToken} = node as ts.BinaryExpression;
const leftName: ts.Identifier | undefined = left && (left as ts.PropertyAccessExpression).name;

super.visitPropertyAccessExpression(node);
}
if (
operatorToken &&
operatorToken.kind === ts.SyntaxKind.EqualsToken &&
left &&
left.kind === ts.SyntaxKind.PropertyAccessExpression &&
leftName &&
right &&
!stringLiteralKinds.includes(right.kind) &&
unsafeElementHtmlProps.includes(leftName.text)
) {
ctx.addFailureAtNode(node, `Found Element.${leftName.text} with non-literal value`);
}

visitBinaryExpression(node: ts.BinaryExpression) {
const left: ts.PropertyAccessExpression = node.left as ts.PropertyAccessExpression;
const right: ts.Expression = node.right;

if (
node.operatorToken &&
node.operatorToken.kind === ts.SyntaxKind.EqualsToken &&
left &&
left.kind === ts.SyntaxKind.PropertyAccessExpression &&
left.name &&
right &&
!stringLiteralKinds.includes(right.kind) &&
unsafeElementHtmlProps.includes(left.name.text)
) {
this.addFailureAtNode(node, `Found Element.${left.name.text} with non-literal value`);
break;
}
default:
//
}

super.visitBinaryExpression(node);
return ts.forEachChild(node, visitNode);
}

return ts.forEachChild(ctx.sourceFile, visitNode);
}
28 changes: 16 additions & 12 deletions src/rules/tsrDetectNoCsrfBeforeMethodOverrideRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,29 @@ 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()));
return this.applyWithFunction(sourceFile, walk);
}
}

class RuleWalker extends Lint.RuleWalker {
private isCsrfFound?: boolean;
function walk(ctx: Lint.WalkContext<void>) {
let isCsrfFound: boolean | undefined;

visitPropertyAccessExpression(node: ts.PropertyAccessExpression) {
const name: ts.Identifier = node.name as ts.Identifier;
const expression: ts.Identifier = node.expression as ts.Identifier;
function visitNode(node: ts.Node): void {
if (node.kind === ts.SyntaxKind.PropertyAccessExpression) {
const {name, expression} = node as ts.PropertyAccessExpression;
const nameText: string | undefined = name && (name as ts.Identifier).text;

if (name && expression && expression.text === 'express') {
if (name.text === 'methodOverride' && this.isCsrfFound) {
this.addFailureAtNode(node, 'express.csrf() middleware found before express.methodOverride()');
} else if (name.text === 'csrf') {
this.isCsrfFound = true;
if (expression && (expression as ts.Identifier).text === 'express') {
if (isCsrfFound && nameText === 'methodOverride') {
ctx.addFailureAtNode(node, 'express.csrf() middleware found before express.methodOverride()');
} else if (nameText === 'csrf') {
isCsrfFound = true;
}
}
}

super.visitPropertyAccessExpression(node);
return ts.forEachChild(node, visitNode);
}

return ts.forEachChild(ctx.sourceFile, visitNode);
}
Loading

0 comments on commit 1186f3f

Please sign in to comment.