Skip to content

Commit

Permalink
Added new detect-child-process rule (microsoft#836)
Browse files Browse the repository at this point in the history
  • Loading branch information
soon committed Apr 4, 2019
1 parent e92a08f commit 3731839
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 0 deletions.
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic
</td>
<td>1.0</td>
</tr>
<tr>
<td>
<code>detect-child-process</code>
</td>
<td>
Detects usages of child_process and especially child_process.exec() with a non-literal first argument.
It is dangerous to pass a string constructed at runtime as the first argument to the child_process.exec().
`child_process.exec(cmd)` runs `cmd` as a shell command which allows attacker to execute malicious code injected into `cmd` string.
Instead of `child_process.exec(cmd)` you should use `child_process.spawn(cmd)` or specify the command as a literal, e.g. `child_process.exec('ls')`.
</td>
<td>@next</td>
</tr>
<tr>
<td>
<code>export-name</code>
Expand Down
1 change: 1 addition & 0 deletions configs/latest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"extends": ["./recommended.json"],
"rulesDirectory": ["../"],
"rules": {
"detect-child-process": true,
"react-a11y-iframes": true,
"void-zero": true
}
Expand Down
1 change: 1 addition & 0 deletions cwe_descriptions.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"75": "Failure to Sanitize Special Elements into a Different Plane (Special Element Injection)",
"79": "Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')",
"85": "Doubled Character XSS Manipulations",
"88": "Argument Injection or Modification",
"95": "Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')",
"116": "Improper Encoding or Escaping of Output",
"157": "Failure to Sanitize Paired Delimiters",
Expand Down
128 changes: 128 additions & 0 deletions src/detectChildProcessRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import * as Lint from 'tslint';
import * as tsutils from 'tsutils';
import * as ts from 'typescript';
import { AstUtils } from './utils/AstUtils';

import { ExtendedMetadata } from './utils/ExtendedMetadata';

const FORBIDDEN_IMPORT_FAILURE_STRING: string = 'Found child_process import';
const FOUND_EXEC_FAILURE_STRING: string = 'Found child_process.exec() with non-literal first argument';

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: ExtendedMetadata = {
ruleName: 'detect-child-process',
type: 'maintainability',
description: 'Tries to detect instances of child_process',
options: null, // tslint:disable-line:no-null-keyword
optionsDescription: '',
typescriptOnly: true,
issueClass: 'SDL',
issueType: 'Error',
severity: 'Important',
level: 'Opportunity for Excellence',
group: 'Security',
commonWeaknessEnumeration: '88'
};

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
}
}

function processImport(
ctx: Lint.WalkContext<void>,
node: ts.Node,
moduleAlias: string | undefined,
importedFunctionsAliases: string[],
importedModuleName: string,
childProcessModuleAliases: Set<string>,
childProcessFunctionAliases: Set<string>
) {
if (importedModuleName === 'child_process') {
ctx.addFailureAt(node.getStart(), node.getWidth(), FORBIDDEN_IMPORT_FAILURE_STRING);
if (moduleAlias !== undefined) {
childProcessModuleAliases.add(moduleAlias);
}
importedFunctionsAliases.forEach(x => childProcessFunctionAliases.add(x));
}
}

function walk(ctx: Lint.WalkContext<void>) {
const childProcessModuleAliases = new Set<string>();
const childProcessFunctionAliases = new Set<string>();

function cb(node: ts.Node): void {
if (tsutils.isImportEqualsDeclaration(node)) {
const alias: string = node.name.text;

if (tsutils.isExternalModuleReference(node.moduleReference)) {
const moduleRef: ts.ExternalModuleReference = node.moduleReference;
if (tsutils.isStringLiteral(moduleRef.expression)) {
const moduleName: string = moduleRef.expression.text;
processImport(ctx, node, alias, [], moduleName, childProcessModuleAliases, childProcessFunctionAliases);
}
}
}

if (tsutils.isImportDeclaration(node)) {
let alias: string | undefined;
let importedNames: string[] = [];

if (node.importClause !== undefined) {
if (node.importClause.name !== undefined) {
alias = tsutils.getIdentifierText(node.importClause.name);
}
if (node.importClause.namedBindings !== undefined) {
if (tsutils.isNamespaceImport(node.importClause.namedBindings)) {
alias = tsutils.getIdentifierText(node.importClause.namedBindings.name);
} else if (tsutils.isNamedImports(node.importClause.namedBindings)) {
importedNames = node.importClause.namedBindings.elements
.filter(x => {
let originalIdentifier: ts.Identifier;

if (x.propertyName === undefined) {
originalIdentifier = x.name;
} else {
originalIdentifier = x.propertyName;
}
return tsutils.getIdentifierText(originalIdentifier) === 'exec';
})
.map(x => tsutils.getIdentifierText(x.name));
}
}
}
if (tsutils.isStringLiteral(node.moduleSpecifier)) {
const moduleName: string = node.moduleSpecifier.text;
processImport(ctx, node, alias, importedNames, moduleName, childProcessModuleAliases, childProcessFunctionAliases);
}
}

if (tsutils.isCallExpression(node)) {
const functionName: string = AstUtils.getFunctionName(node);
const functionTarget = AstUtils.getFunctionTarget(node);

if (functionTarget !== undefined) {
if (
childProcessModuleAliases.has(functionTarget) &&
functionName === 'exec' &&
node.arguments.length === 1 &&
!tsutils.isStringLiteral(node.arguments[0])
) {
ctx.addFailureAt(node.getStart(), node.getWidth(), FOUND_EXEC_FAILURE_STRING);
}
} else {
if (
childProcessFunctionAliases.has(functionName) &&
node.arguments.length === 1 &&
!tsutils.isStringLiteral(node.arguments[0])
) {
ctx.addFailureAt(node.getStart(), node.getWidth(), FOUND_EXEC_FAILURE_STRING);
}
}
}

return ts.forEachChild(node, cb);
}

return ts.forEachChild(ctx.sourceFile, cb);
}
56 changes: 56 additions & 0 deletions tests/detect-child-process/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as child_process from "child_process";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]
import * as child_process_1 from 'child_process';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]
import child_process_2 = require('child_process');
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]

import {exec} from 'child_process';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]
import {spawn} from 'child_process';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]

import {exec as someAnotherExec} from 'child_process';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]
import {spawn as someAnotherSpawn} from 'child_process';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import]

import * as anotherModule from 'anotherModule';


child_process.exec('ls')
child_process.exec(cmd)
~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument]

child_process.spawn('ls')
child_process.spawn(cmd)

child_process_1.exec('ls')
child_process_1.exec(cmd)
~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument]

child_process_1.spawn('ls')
child_process_1.spawn(cmd)

child_process_2.exec('ls')
child_process_2.exec(cmd)
~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument]

child_process_2.spawn('ls')
child_process_2.spawn(cmd)

exec('ls')
exec(cmd)
~~~~~~~~~ [Found child_process.exec() with non-literal first argument]

spawn('ls')
spawn(cmd)

someAnotherExec('ls')
someAnotherExec(cmd)
~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument]

someAnotherSpawn('ls')
someAnotherSpawn(cmd)

anotherModule.exec(cmd)
5 changes: 5 additions & 0 deletions tests/detect-child-process/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"detect-child-process": true
}
}

0 comments on commit 3731839

Please sign in to comment.