Merge pull request github#5621 from erik-krogh/shellSink

Approved by esbena

Author: codeql-ci

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -70,6 +70,12 @@ module UnsafeShellCommandConstruction {
     exists(DataFlow::TypeBackTracker t2 |
       t2 = t.smallstep(result, isExecutedAsShellCommand(t2, sys))
     )
+    or
+    exists(DataFlow::TypeBackTracker t2, StringOps::ConcatenationRoot prev |
+      t = t2.continue() and
+      isExecutedAsShellCommand(t2, sys) = prev and
+      result = prev.getALeaf()
+    )
   }
 
   /**

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

@@ -242,6 +242,10 @@ nodes
 | lib/lib.js:478:27:478:32 | config |
 | lib/lib.js:478:27:478:46 | config.installedPath |
 | lib/lib.js:478:27:478:46 | config.installedPath |
+| lib/lib.js:482:40:482:43 | name |
+| lib/lib.js:482:40:482:43 | name |
+| lib/lib.js:483:30:483:33 | name |
+| lib/lib.js:483:30:483:33 | name |
 | lib/subLib/index.js:3:28:3:31 | name |
 | lib/subLib/index.js:3:28:3:31 | name |
 | lib/subLib/index.js:4:22:4:25 | name |
@@ -538,6 +542,10 @@ edges
 | lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:32 | config |
 | lib/lib.js:478:27:478:32 | config | lib/lib.js:478:27:478:46 | config.installedPath |
 | lib/lib.js:478:27:478:32 | config | lib/lib.js:478:27:478:46 | config.installedPath |
+| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
+| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
+| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
+| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
 | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
@@ -615,5 +623,6 @@ edges
 | lib/lib.js:442:12:442:27 | "rm -rf " + name | lib/lib.js:441:39:441:42 | name | lib/lib.js:442:24:442:27 | name | $@ based on $@ is later used in $@. | lib/lib.js:442:12:442:27 | "rm -rf " + name | String concatenation | lib/lib.js:441:39:441:42 | name | library input | lib/lib.js:442:2:442:28 | asyncEx ... + name) | shell command |
 | lib/lib.js:447:13:447:28 | "rm -rf " + name | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name | $@ based on $@ is later used in $@. | lib/lib.js:447:13:447:28 | "rm -rf " + name | String concatenation | lib/lib.js:446:20:446:23 | name | library input | lib/lib.js:447:3:447:29 | asyncEx ... + name) | shell command |
 | lib/lib.js:478:27:478:46 | config.installedPath | lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:46 | config.installedPath | $@ based on $@ is later used in $@. | lib/lib.js:478:27:478:46 | config.installedPath | Path concatenation | lib/lib.js:477:33:477:38 | config | library input | lib/lib.js:479:12:479:20 | exec(cmd) | shell command |
+| lib/lib.js:483:13:483:33 | ' my na ...  + name | lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name | $@ based on $@ is later used in $@. | lib/lib.js:483:13:483:33 | ' my na ...  + name | String concatenation | lib/lib.js:482:40:482:43 | name | library input | lib/lib.js:485:2:485:20 | cp.exec(cmd + args) | shell command |
 | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:3:28:3:31 | name | library input | lib/subLib/index.js:4:2:4:26 | cp.exec ... + name) | shell command |
 | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:7:32:7:35 | name | library input | lib/subLib/index.js:8:2:8:26 | cp.exec ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js

@@ -477,4 +477,10 @@ const exec = promisify(require('child_process').exec);
 module.exports = function check(config) {
     const cmd = path.join(config.installedPath, 'myBinary -v'); // NOT OK
     return exec(cmd);
+}
+
+module.exports.splitConcat = function (name) {
+	let args = ' my name is ' + name; // NOT OK
+	let cmd = 'echo';
+	cp.exec(cmd + args);
 }