From 80a32aebc1c0271fa106d9286f64048f9751f261 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 4 Oct 2019 11:20:22 +0200 Subject: [PATCH 1/2] JS: add SystemCommandExecution::isShellInterpreted --- .../ql/src/semmle/javascript/Concepts.qll | 3 ++ .../javascript/frameworks/NodeJSLib.qll | 27 +++++++--- .../semmle/javascript/frameworks/ShellJS.qll | 2 + .../frameworks/SystemCommandExecutors.qll | 51 +++++++++++++------ 4 files changed, 59 insertions(+), 24 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/Concepts.qll b/javascript/ql/src/semmle/javascript/Concepts.qll index e0654cd3c9f..3d94723a013 100644 --- a/javascript/ql/src/semmle/javascript/Concepts.qll +++ b/javascript/ql/src/semmle/javascript/Concepts.qll @@ -14,6 +14,9 @@ abstract class SystemCommandExecution extends DataFlow::Node { /** Gets an argument to this execution that specifies the command. */ abstract DataFlow::Node getACommandArgument(); + /** Holds if a shell interprets `arg`. */ + predicate isShellInterpreted(DataFlow::Node arg) { none() } + /** * Gets an argument to this command execution that specifies the argument list * to the command. diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index 71d20546cd9..13a47ce8d8e 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -573,21 +573,32 @@ module NodeJSLib { this = DataFlow::moduleMember("child_process", methodName).getACall() } - override DataFlow::Node getACommandArgument() { + private DataFlow::Node getACommandArgument(boolean shell) { // check whether this is an invocation of an exec/spawn/fork method ( - methodName = "exec" or - methodName = "execSync" or - methodName = "execFile" or - methodName = "execFileSync" or - methodName = "spawn" or - methodName = "spawnSync" or - methodName = "fork" + shell = true and + ( + methodName = "exec" or + methodName = "execSync" + ) + or + shell = false and + ( + methodName = "execFile" or + methodName = "execFileSync" or + methodName = "spawn" or + methodName = "spawnSync" or + methodName = "fork" + ) ) and // all of the above methods take the command as their first argument result = getArgument(0) } + override DataFlow::Node getACommandArgument() { result = getACommandArgument(_) } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = getACommandArgument(true) } + override DataFlow::Node getArgumentList() { ( methodName = "execFile" or diff --git a/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll b/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll index 257663a244d..22605479244 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll @@ -158,6 +158,8 @@ module ShellJS { ShellJSExec() { name = "exec" } override DataFlow::Node getACommandArgument() { result = getArgument(0) } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = getACommandArgument() } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll b/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll index 39dc371f100..7cea87bb3f3 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll @@ -8,18 +8,26 @@ import javascript private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::InvokeNode { int cmdArg; + boolean shell; + SystemCommandExecutors() { exists(string mod, DataFlow::SourceNode callee | exists(string method | - mod = "cross-spawn" and method = "sync" and cmdArg = 0 + mod = "cross-spawn" and method = "sync" and cmdArg = 0 and shell = false or mod = "execa" and ( - method = "shell" or - method = "shellSync" or - method = "stdout" or - method = "stderr" or - method = "sync" + shell = false and + ( + method = "shell" or + method = "shellSync" or + method = "stdout" or + method = "stderr" or + method = "sync" + ) + or + shell = true and + (method = "command" or method = "commandSync") ) and cmdArg = 0 | @@ -27,17 +35,24 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I ) or ( - mod = "cross-spawn" and cmdArg = 0 + shell = false and + ( + mod = "cross-spawn" and cmdArg = 0 + or + mod = "cross-spawn-async" and cmdArg = 0 + or + mod = "exec-async" and cmdArg = 0 + or + mod = "execa" and cmdArg = 0 + ) or - mod = "cross-spawn-async" and cmdArg = 0 - or - mod = "exec" and cmdArg = 0 - or - mod = "exec-async" and cmdArg = 0 - or - mod = "execa" and cmdArg = 0 - or - mod = "remote-exec" and cmdArg = 1 + shell = true and + ( + mod = "exec" and + cmdArg = 0 + or + mod = "remote-exec" and cmdArg = 1 + ) ) and callee = DataFlow::moduleImport(mod) | @@ -46,4 +61,8 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I } override DataFlow::Node getACommandArgument() { result = getArgument(cmdArg) } + + override predicate isShellInterpreted(DataFlow::Node arg) { + arg = getACommandArgument() and shell = true + } } From 5a983cb535151d176dc4c0e7bc6ce863249835f9 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 4 Oct 2019 11:22:07 +0200 Subject: [PATCH 2/2] JS: add query js/shell-command-injection-from-environment --- change-notes/1.23/analysis-javascript.md | 1 + javascript/config/suites/javascript/security | 1 + ...ShellCommandInjectionFromEnvironment.qhelp | 103 ++++++++++++++++++ .../ShellCommandInjectionFromEnvironment.ql | 29 +++++ ...hell-command-injection-from-environment.js | 6 + ...ommand-injection-from-environment_fixed.js | 7 ++ .../ShellCommandInjectionFromEnvironment.qll | 34 ++++++ ...InjectionFromEnvironmentCustomizations.qll | 58 ++++++++++ .../CWE-078/CommandInjection.expected | 1 + .../CWE-078/IndirectCommandInjection.expected | 1 + ...llCommandInjectionFromEnvironment.expected | 46 ++++++++ ...ShellCommandInjectionFromEnvironment.qlref | 1 + ...hell-command-injection-from-environment.js | 6 + 13 files changed, 294 insertions(+) create mode 100644 javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.qhelp create mode 100644 javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.ql create mode 100644 javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js create mode 100644 javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment_fixed.js create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironment.qll create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironmentCustomizations.qll create mode 100644 javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.expected create mode 100644 javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.qlref create mode 100644 javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js diff --git a/change-notes/1.23/analysis-javascript.md b/change-notes/1.23/analysis-javascript.md index db18499cc68..2a86d53b0a0 100644 --- a/change-notes/1.23/analysis-javascript.md +++ b/change-notes/1.23/analysis-javascript.md @@ -17,6 +17,7 @@ | Unused index variable (`js/unused-index-variable`) | correctness | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. Results are shown on LGTM by default. | | Loop bound injection (`js/loop-bound-injection`) | security, external/cwe/cwe-834 | Highlights loops where a user-controlled object with an arbitrary .length value can trick the server to loop indefinitely. Results are not shown on LGTM by default. | | Suspicious method name (`js/suspicious-method-name-declaration`) | correctness, typescript, methods | Highlights suspiciously named methods where the developer likely meant to write a constructor or function. Results are shown on LGTM by default. | +| Shell command built from environment values (`js/shell-command-injection-from-environment`) | correctness, security, external/cwe/cwe-078, external/cwe/cwe-088 | Highlights shell commands that may change behavior inadvertently depending on the execution environment, indicating a possible violation of [CWE-78](https://cwe.mitre.org/data/definitions/78.html). Results are shown on LGTM by default.| | Use of returnless function (`js/use-of-returnless-function`) | maintainability, correctness | Highlights calls where the return value is used, but the callee never returns a value. Results are shown on LGTM by default. | | Useless regular expression character escape (`js/useless-regexp-character-escape`) | correctness, security, external/cwe/cwe-20 | Highlights regular expression strings with useless character escapes, indicating a possible violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. | diff --git a/javascript/config/suites/javascript/security b/javascript/config/suites/javascript/security index 7a3b93d1b2e..5dbd0a81811 100644 --- a/javascript/config/suites/javascript/security +++ b/javascript/config/suites/javascript/security @@ -9,6 +9,7 @@ + semmlecode-javascript-queries/Security/CWE-022/ZipSlip.ql: /Security/CWE/CWE-022 + semmlecode-javascript-queries/Security/CWE-078/CommandInjection.ql: /Security/CWE/CWE-078 + semmlecode-javascript-queries/Security/CWE-078/IndirectCommandInjection.ql: /Security/CWE/CWE-078 ++ semmlecode-javascript-queries/Security/CWE-078/ShellCommandInjectionFromEnvironment: /Security/CWE/CWE-078 + semmlecode-javascript-queries/Security/CWE-079/ReflectedXss.ql: /Security/CWE/CWE-079 + semmlecode-javascript-queries/Security/CWE-079/StoredXss.ql: /Security/CWE/CWE-079 + semmlecode-javascript-queries/Security/CWE-079/Xss.ql: /Security/CWE/CWE-079 diff --git a/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.qhelp b/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.qhelp new file mode 100644 index 00000000000..8937b67a420 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.qhelp @@ -0,0 +1,103 @@ + + + +

+ + Dynamically constructing a shell command with values from the + local environment, such as file paths, may inadvertently + change the meaning of the shell command. + + Such changes can occur when an environment value contains + characters that the shell interprets in a special way, for instance + quotes and spaces. + + This can result in the shell command misbehaving, or even + allowing a malicious user to execute arbitrary commands on the system. + +

+ + + + + +

+ + If possible, use hard-coded string literals to specify the + shell command to run, and provide the dynamic arguments to the shell + command separately to avoid interpretation by the shell. + +

+ +

+ + Alternatively, if the shell command must be constructed + dynamically, then add code to ensure that special characters in + environment values do not alter the shell command unexpectedly. + +

+ + + + +

+ + The following example shows a dynamically constructed shell + command that recursively removes a temporary directory that is located + next to the currently executing JavaScript file. Such utilities are + often found in custom build scripts. + +

+ + + +

+ + The shell command will, however, fail to work as intended if the + absolute path of the script's directory contains spaces. In that + case, the shell command will interpret the absolute path as multiple + paths, instead of a single path. +

+ +

+ + For instance, if the absolute path of + the temporary directory is /home/username/important + project/temp, then the shell command will recursively delete + /home/username/important and project/temp, + where the latter path gets resolved relative to the working directory + of the JavaScript process. + +

+ + +

+ + Even worse, although less likely, a malicious user could + provide the path /home/username/; cat /etc/passwd #/important + project/temp in order to execute the command cat + /etc/passwd. + +

+ +

+ + To avoid such potentially catastrophic behaviors, provide the + directory as an argument that does not get interpreted by a + shell: + +

+ + + + + + +
  • + OWASP: + Command Injection. +
    • + +     +     diff --git a/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.ql b/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.ql new file mode 100644 index 00000000000..a5e51b1c71d --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/ShellCommandInjectionFromEnvironment.ql @@ -0,0 +1,29 @@ +/** + * @name Shell command built from environment values + * @description Building a shell command string with values from the enclosing + * environment may cause subtle bugs or vulnerabilities. + * @kind path-problem + * @problem.severity warning + * @precision high + * @id js/shell-command-injection-from-environment + * @tags correctness + * security + * external/cwe/cwe-078 + * external/cwe/cwe-088 + */ + +import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironment::ShellCommandInjectionFromEnvironment + +from + Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node highlight, + Source sourceNode +where + sourceNode = source.getNode() and + cfg.hasFlowPath(source, sink) and + if cfg.isSinkWithHighlight(sink.getNode(), _) + then cfg.isSinkWithHighlight(sink.getNode(), highlight) + else highlight = sink.getNode() +select highlight, source, sink, "This shell command depends on an uncontrolled $@.", sourceNode, + sourceNode.getSourceType() diff --git a/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js new file mode 100644 index 00000000000..3bc4436a5e5 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment.js @@ -0,0 +1,6 @@ +var cp = require("child_process"), + path = require("path"); +function cleanupTemp() { + let cmd = "rm -rf " + path.join(__dirname, "temp"); + cp.execSync(cmd); // BAD +} diff --git a/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment_fixed.js b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment_fixed.js new file mode 100644 index 00000000000..a9b8820f091 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/examples/shell-command-injection-from-environment_fixed.js @@ -0,0 +1,7 @@ +var cp = require("child_process"), + path = require("path"); +function cleanupTemp() { + let cmd = "rm", + args = ["-rf", path.join(__dirname, "temp")]; + cp.execFileSync(cmd, args); // GOOD +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironment.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironment.qll new file mode 100644 index 00000000000..a47f593612f --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironment.qll @@ -0,0 +1,34 @@ +/** + * Provides a taint tracking configuration for reasoning about + * command-injection vulnerabilities (CWE-078). + * + * Note, for performance reasons: only import this file if + * `ShellCommandInjectionFromEnvironment::Configuration` is needed, otherwise + * `ShellCommandInjectionFromEnvironmentCustomizations` should be imported instead. + */ + +import javascript + +module ShellCommandInjectionFromEnvironment { + import ShellCommandInjectionFromEnvironmentCustomizations::ShellCommandInjectionFromEnvironment + import IndirectCommandArgument + + /** + * A taint-tracking configuration for reasoning about command-injection vulnerabilities. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "ShellCommandInjectionFromEnvironment" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + predicate isSinkWithHighlight(DataFlow::Node sink, DataFlow::Node highlight) { + sink instanceof Sink and highlight = sink + or + isIndirectCommandArgument(sink, highlight) + } + + override predicate isSink(DataFlow::Node sink) { isSinkWithHighlight(sink, _) } + + override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironmentCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironmentCustomizations.qll new file mode 100644 index 00000000000..4528376b1c8 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironmentCustomizations.qll @@ -0,0 +1,58 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * command-injection vulnerabilities, as well as extension points for + * adding your own. + */ + +import javascript +import semmle.javascript.security.dataflow.TaintedPathCustomizations + +module ShellCommandInjectionFromEnvironment { + /** + * A data flow source for command-injection vulnerabilities. + */ + abstract class Source extends DataFlow::Node { + /** Gets a string that describes the type of this data flow source. */ + abstract string getSourceType(); + } + + /** + * A data flow sink for command-injection vulnerabilities. + */ + abstract class Sink extends DataFlow::Node { } + + /** + * A sanitizer for command-injection vulnerabilities. + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** An file name from the local file system, considered as a flow source for command injection. */ + class FileNameSourceAsSource extends Source { + FileNameSourceAsSource() { this instanceof FileNameSource } + + override string getSourceType() { result = "file name" } + } + + /** An absolute path from the local file system, considered as a flow source for command injection. */ + class AbsolutePathSource extends Source { + AbsolutePathSource() { + exists(ModuleScope ms | this.asExpr() = ms.getVariable("__dirname").getAnAccess()) + or + exists(DataFlow::SourceNode process | process = NodeJSLib::process() | + this = process.getAPropertyRead("execPath") or + this = process.getAMemberCall("cwd") + ) + or + this = any(TaintedPath::ResolvingPathCall c).getOutput() + } + + override string getSourceType() { result = "absolute path" } + } + + /** + * A shell command argument. + */ + class ShellCommandSink extends Sink, DataFlow::ValueNode { + ShellCommandSink() { any(SystemCommandExecution sys).isShellInterpreted(this) } + } +} diff --git a/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected b/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected index 0cc0f75435d..8b058d2e799 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected @@ -69,6 +69,7 @@ nodes | other.js:19:36:19:38 | cmd | | third-party-command-injection.js:5:20:5:26 | command | | third-party-command-injection.js:6:21:6:27 | command | +| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] | edges | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd | | child_process-test.js:6:9:6:49 | cmd | child_process-test.js:18:17:18:19 | cmd | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection.expected b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection.expected index 86f47ee6a6d..bd142297996 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection.expected @@ -54,6 +54,7 @@ nodes | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` | | command-line-parameter-command-injection.js:27:32:27:35 | args | | command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') | +| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] | edges | child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:5:39:5 | sh | | child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.expected b/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.expected new file mode 100644 index 00000000000..9015113f9fb --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.expected @@ -0,0 +1,46 @@ +nodes +| child_process-test.js:36:7:36:20 | sh | +| child_process-test.js:36:12:36:20 | 'cmd.exe' | +| child_process-test.js:38:7:38:20 | sh | +| child_process-test.js:38:12:38:20 | '/bin/sh' | +| child_process-test.js:39:14:39:15 | sh | +| child_process-test.js:39:18:39:30 | [ flag, cmd ] | +| child_process-test.js:41:9:41:17 | args | +| child_process-test.js:41:16:41:17 | [] | +| child_process-test.js:44:17:44:27 | "/bin/bash" | +| child_process-test.js:44:30:44:33 | args | +| child_process-test.js:46:9:46:12 | "sh" | +| child_process-test.js:46:15:46:18 | args | +| child_process-test.js:48:9:48:17 | args | +| child_process-test.js:48:16:48:17 | [] | +| child_process-test.js:51:17:51:32 | `/bin` + "/bash" | +| child_process-test.js:51:35:51:38 | args | +| child_process-test.js:55:14:55:16 | cmd | +| child_process-test.js:55:19:55:22 | args | +| child_process-test.js:56:12:56:14 | cmd | +| child_process-test.js:56:17:56:20 | args | +| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] | +| tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | +| tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") | +| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | +edges +| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:5:39:5 | sh | +| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh | +| child_process-test.js:36:12:36:20 | 'cmd.exe' | child_process-test.js:36:7:36:20 | sh | +| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:5:39:5 | sh | +| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:14:39:15 | sh | +| child_process-test.js:38:12:38:20 | '/bin/sh' | child_process-test.js:38:7:38:20 | sh | +| child_process-test.js:39:5:39:5 | sh | child_process-test.js:39:14:39:15 | sh | +| child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args | +| child_process-test.js:41:9:41:17 | args | child_process-test.js:46:15:46:18 | args | +| child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args | +| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd | +| child_process-test.js:46:15:46:18 | args | child_process-test.js:55:19:55:22 | args | +| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args | +| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args | +| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd | +| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args | +| tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | +| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") | +#select +| tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | This shell command depends on an uncontrolled $@. | tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | absolute path | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.qlref b/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.qlref new file mode 100644 index 00000000000..ee13f263562 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/ShellCommandInjectionFromEnvironment.qlref @@ -0,0 +1 @@ +Security/CWE-078/ShellCommandInjectionFromEnvironment.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js new file mode 100644 index 00000000000..08f71a8ee0c --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js @@ -0,0 +1,6 @@ +var cp = require('child_process'), + path = require('path'); +(function() { + cp.execFileSync('rm', ['-rf', path.join(__dirname, "temp")]); // GOOD + cp.execSync('rm -rf ' + path.join(__dirname, "temp")); // BAD +});