From 9657491d331f50606c27c872114eb359e9541c24 Mon Sep 17 00:00:00 2001 From: amammad Date: Thu, 21 Sep 2023 06:02:17 +1000 Subject: [PATCH] V1 --- .../semmle/javascript/frameworks/Execa.qll | 64 +++++++------------ .../frameworks/Execa/Execa.expected | 12 ++-- .../library-tests/frameworks/Execa/tst.js | 12 ++-- 3 files changed, 38 insertions(+), 50 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll b/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll index 5d770d1eed72..24574ac32853 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll @@ -88,12 +88,6 @@ module Execa { isExecaShellEnable(this.getParameter(1)) } - predicate isArgumentInjectable(DataFlow::Node arg) { - // execa(file: string,arguments?: readonly string[],options?: Options) - arg = this.getParameter(1).getUnknownMember().asSink() and - argumentIsInjectable(this.getParameter(0).asSink().getStringValue()) - } - override predicate isSync() { name = "execaSync" } override DataFlow::Node getOptionsArg() { @@ -134,7 +128,7 @@ module Execa { ExecaScriptEec() { name = ["Sync", "ASync"] } override DataFlow::Node getACommandArgument() { - result.asExpr() = templateLiteralChildAsSink(this.asExpr()).getAChildExpr+() + result.asExpr() = templateLiteralChildAsSink(this.asExpr()).getChildExpr(0) } override predicate isShellInterpreted(DataFlow::Node arg) { @@ -179,13 +173,32 @@ module Execa { string getName() { result = name } } + /** + * The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions + */ + class ExecaCommandExec2 extends SystemCommandExecution, DataFlow::CallNode { + ExecaCommandExec2() { this = API::moduleImport("execa").getMember("execaCommand").getACall() } + + override DataFlow::Node getACommandArgument() { result = this.getArgument(0) } + + override DataFlow::Node getArgumentList() { result = this.getArgument(0) } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getArgument(0) } + + override predicate isSync() { none() } + + override DataFlow::Node getOptionsArg() { result = this } + } + /** * The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions */ class ExecaCommandExec extends SystemCommandExecution, ExecaCommandCall { ExecaCommandExec() { name = ["execaCommand", "execaCommandSync"] } - override DataFlow::Node getACommandArgument() { result = this.getArgument(0) } + override DataFlow::Node getACommandArgument() { + result = this.(DataFlow::CallNode).getArgument(0) + } override DataFlow::Node getArgumentList() { // execaCommand("echo " + sink); @@ -212,7 +225,7 @@ module Execa { } } - // parent = left`result` + // Holds if left parameter is the the left child of a template literal and returns the template literal private TemplateLiteral templateLiteralChildAsSink(Expr left) { exists(TaggedTemplateExpr parent | parent.getTemplate() = result and @@ -220,42 +233,13 @@ module Execa { ) } - private class CommandsVulnerableToArgumentInjection extends string { - CommandsVulnerableToArgumentInjection() { - // Thanks to https://sonarsource.github.io/argument-injection-vectors/#+command - this = - [ - "chrome", "git-blame", "git-clone", "git-fetch", "git-grep", "git-ls-remote", "hg", - "psql", "qt5", "ssh", "tar", "zip", "aria2c", "tcpdump", "sysctl", "split", "sed", - "pidstat", "php", "nohup", "crontab", "crontab", "crontab", "crontab", "sh", "zsh", - "bash", "cmd.exe", "cmd" - ] - } - } - - // Check whether a command is vulnerable to argument injection or not - bindingset[cmd] - private predicate argumentIsInjectable(string cmd) { - // "cmd args" or "cmd" - exists(CommandsVulnerableToArgumentInjection c | - // full/relative path to command like `/usr/bin/cmd` or `someDir/cmd` - cmd.matches("%/" + c) - or - // command like `cmd args` - cmd.matches(c + " %") - or - // only the command `cmd` - cmd = c - ) - } - - // Check whether Execa has shell enabled options or not, get Parameter responsible for options + // Holds whether Execa has shell enabled options or not, get Parameter responsible for options private predicate isExecaShellEnable(API::Node n) { n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true" and exists(n.getMember("shell")) } - // Check whether Execa has shell enabled options or not, get Parameter responsible for options + // Holds whether Execa has shell enabled options or not, get Parameter responsible for options private predicate isExecaShellEnableWithExpr(Expr n) { exists(ObjectExpr o, Property p | o = n.getAChildExpr*() | o.getAChild() = p and diff --git a/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected b/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected index 7c27ecc42153..a99d033b6efc 100644 --- a/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected +++ b/javascript/ql/test/library-tests/frameworks/Execa/Execa.expected @@ -17,9 +17,9 @@ test_FileSystemAccess | tst.js:37:18:37:20 | arg | | tst.js:39:18:39:39 | "echo 1 ... echo 2" | | tst.js:39:42:39:56 | { shell: true } | -| tst.js:45:9:45:29 | { input ... sink' } | +| tst.js:45:9:45:27 | { inputFile: file } | | tst.js:46:13:46:17 | 'cat' | -| tst.js:46:20:46:40 | { input ... sink' } | +| tst.js:46:20:46:38 | { inputFile: file } | | tst.js:47:13:47:18 | 'echo' | | tst.js:47:21:47:32 | ['example2'] | | tst.js:48:13:48:18 | 'echo' | @@ -28,6 +28,10 @@ test_FileSystemAccess | tst.js:49:21:49:32 | ['example4'] | | tst.js:49:35:49:47 | { all: true } | test_MissingFileSystemAccess +| tst.js:43:35:43:38 | file | +| tst.js:47:46:47:49 | file | +| tst.js:48:46:48:49 | file | +| tst.js:49:58:49:61 | file | test_SystemCommandExecution | tst.js:1:71:1:71 | $ | | tst.js:4:7:4:7 | $ | @@ -56,8 +60,8 @@ test_SystemCommandExecution | tst.js:39:1:39:57 | execaCo ... true }) | | tst.js:43:7:43:7 | $ | | tst.js:45:7:45:7 | $ | -| tst.js:45:7:45:30 | $({ inp ... ink' }) | -| tst.js:46:7:46:41 | execa(' ... ink' }) | +| tst.js:45:7:45:28 | $({ inp ... file }) | +| tst.js:46:7:46:39 | execa(' ... file }) | | tst.js:47:7:47:33 | execa(' ... ple2']) | | tst.js:48:7:48:33 | execa(' ... ple3']) | | tst.js:49:7:49:48 | execa(' ... true }) | diff --git a/javascript/ql/test/library-tests/frameworks/Execa/tst.js b/javascript/ql/test/library-tests/frameworks/Execa/tst.js index 6b2e59b57d61..0e657d421a24 100644 --- a/javascript/ql/test/library-tests/frameworks/Execa/tst.js +++ b/javascript/ql/test/library-tests/frameworks/Execa/tst.js @@ -40,10 +40,10 @@ execaCommandSync("echo 1 " + "; echo 2", { shell: true }); // FileSystemAccess // Piping stdout to a file -await $`echo example8`.pipeStdout('tmp') +await $`echo example8`.pipeStdout(file) // Piping stdin from a file -await $({ inputFile: 'sink' })`cat` -await execa('cat', { inputFile: 'sink' }); -await execa('echo', ['example2']).pipeStdout('tmpdir/sink.txt'); -await execa('echo', ['example3']).pipeStderr('tmpdir/sink.txt'); -await execa('echo', ['example4'], { all: true }).pipeAll('tmpdir/sink.txt'); +await $({ inputFile: file })`cat` +await execa('cat', { inputFile: file }); +await execa('echo', ['example2']).pipeStdout(file); +await execa('echo', ['example3']).pipeStderr(file); +await execa('echo', ['example4'], { all: true }).pipeAll(file);