From b8194214f6ef14a1fddd30e7bf18747880ce70b7 Mon Sep 17 00:00:00 2001 From: Andrew <0coming.soon@gmail.com> Date: Wed, 24 Apr 2019 17:19:55 +0500 Subject: [PATCH] Added new `detect-child-process` rule (#252) (#855) * Added new `detect-child-process` rule (#252) * Minor docs changes (#252) --- README.md | 12 ++ configs/latest.json | 1 + cwe_descriptions.json | 1 + src/detectChildProcessRule.ts | 206 ++++++++++++++++++++++++ tests/detect-child-process/test.ts.lint | 122 ++++++++++++++ tests/detect-child-process/tslint.json | 5 + 6 files changed, 347 insertions(+) create mode 100644 src/detectChildProcessRule.ts create mode 100644 tests/detect-child-process/test.ts.lint create mode 100644 tests/detect-child-process/tslint.json diff --git a/README.md b/README.md index c766a9851..31a2377cb 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,18 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic 1.0 + + + detect-child-process + + + 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 could allow an attacker to execute malicious code injected into cmd. + 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'). + + @next + export-name diff --git a/configs/latest.json b/configs/latest.json index 3ba4abbd6..7c43cb19c 100644 --- a/configs/latest.json +++ b/configs/latest.json @@ -2,6 +2,7 @@ "extends": ["./recommended.json"], "rulesDirectory": ["../"], "rules": { + "detect-child-process": true, "react-a11y-iframes": true, "react-a11y-mouse-event-has-key-event": true, "void-zero": true diff --git a/cwe_descriptions.json b/cwe_descriptions.json index 2ed462083..e538d73fa 100644 --- a/cwe_descriptions.json +++ b/cwe_descriptions.json @@ -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", diff --git a/src/detectChildProcessRule.ts b/src/detectChildProcessRule.ts new file mode 100644 index 000000000..3a75a9fac --- /dev/null +++ b/src/detectChildProcessRule.ts @@ -0,0 +1,206 @@ +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'; +const FORBIDDEN_MODULE_NAME = 'child_process'; +const FORBIDDEN_FUNCTION_NAME = 'exec'; + +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: ExtendedMetadata = { + ruleName: 'detect-child-process', + type: 'maintainability', + description: 'Detects instances of child_process and child_process.exec', + rationale: Lint.Utils.dedent` + 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'). + `, + 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 getProhibitedImportedNames(namedImports: ts.NamedImports) { + return namedImports.elements + .filter(x => { + let originalIdentifier: ts.Identifier; + + if (x.propertyName === undefined) { + originalIdentifier = x.name; + } else { + originalIdentifier = x.propertyName; + } + return tsutils.getIdentifierText(originalIdentifier) === FORBIDDEN_FUNCTION_NAME; + }) + .map(x => tsutils.getIdentifierText(x.name)); +} + +function isNotUndefined(value: TValue | undefined): value is TValue { + return value !== undefined; +} + +function getProhibitedBoundNames(namedBindings: ts.ObjectBindingPattern) { + return namedBindings.elements + .filter(x => { + if (!ts.isIdentifier(x.name)) { + return false; + } + let importedName: string | undefined; + + if (x.propertyName === undefined) { + importedName = tsutils.getIdentifierText(x.name); + } else { + if (ts.isIdentifier(x.propertyName)) { + importedName = tsutils.getIdentifierText(x.propertyName); + } else if (ts.isStringLiteral(x.propertyName)) { + importedName = x.propertyName.text; + } + } + return importedName === FORBIDDEN_FUNCTION_NAME; + }) + .map(x => { + if (ts.isIdentifier(x.name)) { + return tsutils.getIdentifierText(x.name); + } + return undefined; + }) + .filter(isNotUndefined); +} + +function walk(ctx: Lint.WalkContext) { + const childProcessModuleAliases = new Set(); + const childProcessFunctionAliases = new Set(); + + function processImport(node: ts.Node, moduleAlias: string | undefined, importedFunctionsAliases: string[], importedModuleName: string) { + if (importedModuleName === FORBIDDEN_MODULE_NAME) { + ctx.addFailureAt(node.getStart(), node.getWidth(), FORBIDDEN_IMPORT_FAILURE_STRING); + if (moduleAlias !== undefined) { + childProcessModuleAliases.add(moduleAlias); + } + importedFunctionsAliases.forEach(x => childProcessFunctionAliases.add(x)); + } + } + + function processRequire(node: ts.CallExpression) { + const functionTarget = AstUtils.getFunctionTarget(node); + + if (functionTarget !== undefined || node.arguments.length === 0) { + return; + } + + const firstArg = node.arguments[0]; + if (tsutils.isStringLiteral(firstArg) && firstArg.text === FORBIDDEN_MODULE_NAME) { + let alias: string | undefined; + let importedNames: string[] = []; + + if (tsutils.isVariableDeclaration(node.parent)) { + if (tsutils.isIdentifier(node.parent.name)) { + alias = tsutils.getIdentifierText(node.parent.name); + } else if (tsutils.isObjectBindingPattern(node.parent.name)) { + importedNames = getProhibitedBoundNames(node.parent.name); + } + } + + processImport(node, alias, importedNames, firstArg.text); + } + } + + function isProhibitedCall(node: ts.CallExpression): boolean { + const functionName: string = AstUtils.getFunctionName(node); + const functionTarget = AstUtils.getFunctionTarget(node); + const hasNonStringLiteralFirstArgument = node.arguments.length > 0 && !tsutils.isStringLiteral(node.arguments[0]); + + if (functionTarget === undefined) { + return childProcessFunctionAliases.has(functionName) && hasNonStringLiteralFirstArgument; + } + + return ( + childProcessModuleAliases.has(functionTarget) && functionName === FORBIDDEN_FUNCTION_NAME && hasNonStringLiteralFirstArgument + ); + } + + function processCallExpression(node: ts.CallExpression) { + const functionName: string = AstUtils.getFunctionName(node); + + if (functionName === 'require') { + processRequire(node); + } + + if (isProhibitedCall(node)) { + ctx.addFailureAt(node.getStart(), node.getWidth(), FOUND_EXEC_FAILURE_STRING); + } + } + + function processImportDeclaration(node: ts.ImportDeclaration) { + if (!tsutils.isStringLiteral(node.moduleSpecifier)) { + return; + } + + const moduleName: string = node.moduleSpecifier.text; + + 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 = getProhibitedImportedNames(node.importClause.namedBindings); + } + } + } + + processImport(node, alias, importedNames, moduleName); + } + + function processImportEqualsDeclaration(node: ts.ImportEqualsDeclaration) { + if (tsutils.isExternalModuleReference(node.moduleReference)) { + const moduleRef: ts.ExternalModuleReference = node.moduleReference; + if (tsutils.isStringLiteral(moduleRef.expression)) { + const moduleName: string = moduleRef.expression.text; + const alias: string = node.name.text; + processImport(node, alias, [], moduleName); + } + } + } + + function cb(node: ts.Node): void { + if (tsutils.isImportEqualsDeclaration(node)) { + processImportEqualsDeclaration(node); + } + + if (tsutils.isImportDeclaration(node)) { + processImportDeclaration(node); + } + + if (tsutils.isCallExpression(node)) { + processCallExpression(node); + } + + return ts.forEachChild(node, cb); + } + + return ts.forEachChild(ctx.sourceFile, cb); +} diff --git a/tests/detect-child-process/test.ts.lint b/tests/detect-child-process/test.ts.lint new file mode 100644 index 000000000..c41fc58e1 --- /dev/null +++ b/tests/detect-child-process/test.ts.lint @@ -0,0 +1,122 @@ +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] +const child_process_3 = 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] + +const {exec} = require("child_process"); + ~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import] +const {spawn} = require("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] + +const {exec: someAnotherExec2} = require("child_process"); + ~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import] +const {spawn: someAnotherSpawn2} = require("child_process"); + ~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process import] + +import * as anotherModule from 'anotherModule'; + + +child_process.exec('ls') +child_process.exec('ls', options) +child_process.exec('ls', options, callback) + +child_process.exec(cmd) +~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process.exec(cmd, options) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process.exec(cmd, options, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [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('ls', options) +child_process_1.exec('ls', options, callback) + +child_process_1.exec(cmd) +~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process_1.exec(cmd, options) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process_1.exec(cmd, options, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [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('ls', options) +child_process_2.exec('ls', options, callback) + +child_process_2.exec(cmd) +~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process_2.exec(cmd, options) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +child_process_2.exec(cmd, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] + + +child_process_2.spawn('ls') +child_process_2.spawn(cmd) + +exec('ls') +exec('ls', options) +exec('ls', options, callback) + +exec(cmd) +~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +exec(cmd, options) +~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +exec(cmd, options, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] + +spawn('ls') +spawn(cmd) + +someAnotherExec('ls') +someAnotherExec('ls', options) +someAnotherExec('ls', options, callback) + +someAnotherExec(cmd) +~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +someAnotherExec(cmd, options) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +someAnotherExec(cmd, options, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] + +someAnotherSpawn('ls') +someAnotherSpawn(cmd) + +someAnotherExec2('ls') +someAnotherExec2('ls', options) +someAnotherExec2('ls', options, callback) + +someAnotherExec2(cmd) +~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +someAnotherExec2(cmd, options) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] +someAnotherExec2(cmd, options, callback) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found child_process.exec() with non-literal first argument] + +someAnotherSpawn2('ls') +someAnotherSpawn2(cmd) + +anotherModule.exec(cmd) +anotherModule.exec(cmd, param2) +anotherModule.exec(cmd, param2, param3) + diff --git a/tests/detect-child-process/tslint.json b/tests/detect-child-process/tslint.json new file mode 100644 index 000000000..165761eeb --- /dev/null +++ b/tests/detect-child-process/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "detect-child-process": true + } +} \ No newline at end of file