diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll b/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll index a851f6099ce..db504b06c60 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Execa.qll @@ -119,8 +119,6 @@ module Execa { } } - API::Node test() { result = API::moduleImport("execa").getMember("$").getASuccessor*() } - /** * The system command execution nodes for `execa.$` or `execa.$.sync` tag functions */ @@ -131,16 +129,17 @@ module Execa { override predicate isShellInterpreted(DataFlow::Node arg) { isExecaShellEnable(this.getParameter(0)) and - arg = this.getParameter(0).asSink() + arg = this.getAParameter().asSink() } override DataFlow::Node getArgumentList() { - result = this.getParameter(any(int i | i > 1)).asSink() + result = this.getParameter(any(int i | i > 1)).asSink() and + not exists(string s | this.getACall().getArgument(0).mayHaveStringValue(s) | s.matches("")) } - override predicate isSync() { isSync = true } - override DataFlow::Node getOptionsArg() { result = this.getParameter(0).asSink() } + + override predicate isSync() { isSync = true } } /** diff --git a/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/execa.js b/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/execa.js index 936a6910de4..5155b228550 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/execa.js +++ b/javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/execa.js @@ -4,22 +4,32 @@ import url from 'url' http.createServer(async function (req, res) { let cmd = url.parse(req.url, true).query["cmd"][0]; - let arg = url.parse(req.url, true).query["arg"]; + let arg1 = url.parse(req.url, true).query["arg1"]; + let arg2 = url.parse(req.url, true).query["arg2"]; - await $`${cmd} ${arg}`; // NOT OK - $.sync`${cmd} ${arg}`; // NOT OK - await $({ shell: true })`${cmd} ${arg}` // NOT OK - await $({ shell: false })`${cmd} ${arg}` // NOT OK + await $`${cmd} ${arg1} ${arg2}`; // NOT OK + await $`ssh ${arg1} ${arg2}`; // NOT OK + $({ shell: false }).sync`${cmd} ${arg} ${arg} ${arg2}`; // NOT OK + $({ shell: true }).sync`${cmd} ${arg} ${arg} ${arg2}`; // NOT OK + $({ shell: false }).sync`ssh ${arg} ${arg} ${arg2}`; // NOT OK - await execa(cmd, [arg]); // NOT OK + $.sync`${cmd} ${arg1} ${arg2}`; // NOT OK + $.sync`ssh ${arg1} ${arg2}`; // NOT OK + await $({ shell: true })`${cmd} ${arg1} ${arg2}` // NOT OK + await $({ shell: false })`${cmd} ${arg1} ${arg2}` // NOT OK + await $({ shell: false })`ssh ${arg1} ${arg2}` // NOT OK + + await execa(cmd, [arg1]); // NOT OK await execa(cmd, { shell: true }); // NOT OK await execa(cmd, { shell: true }); // NOT OK - await execa(cmd, [arg], { shell: true }); // NOT OK - execaSync(cmd, [arg]); // NOT OK - execaSync(cmd, [arg], { shell: true }); // NOT OK + await execa(cmd, [arg1], { shell: true }); // NOT OK - await execaCommand(cmd + arg); // NOT OK - execaCommandSync(cmd + arg); // NOT OK - await execaCommand(cmd + arg, { shell: true }); // NOT OK - execaCommandSync(cmd + arg, { shell: true }); // NOT OK + execaSync(cmd, [arg1]); // NOT OK + execaSync(cmd, [arg1], { shell: true }); // NOT OK + + await execaCommand(cmd + arg1); // NOT OK + await execaCommand(cmd + arg1, { shell: true }); // NOT OK + + execaCommandSync(cmd + arg1); // NOT OK + execaCommandSync(cmd + arg1, { shell: true }); // NOT OK }); \ No newline at end of file