Merge pull request github#5526 from erik-krogh/quotedShell

Approved by esbena

Author: codeql-ci

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

@@ -55,4 +55,14 @@ module ShellCommandInjectionFromEnvironment {
   class ShellCommandSink extends Sink, DataFlow::ValueNode {
     ShellCommandSink() { any(SystemCommandExecution sys).isShellInterpreted(this) }
   }
+
+  /**
+   * A string-concatenation leaf that is surrounded by quotes, seen as a sanitizer for command-injection.
+   */
+  class QuotingConcatSanitizer extends Sanitizer, StringOps::ConcatenationLeaf {
+    QuotingConcatSanitizer() {
+      this.getNextLeaf().getStringValue().regexpMatch("(\"|').*") and
+      this.getPreviousLeaf().getStringValue().regexpMatch(".*(\"|')")
+    }
+  }
 }

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

@@ -63,6 +63,7 @@ syncCommand
 | tst_shell-command-injection-from-environment.js:5:2:5:62 | cp.exec ... emp")]) |
 | tst_shell-command-injection-from-environment.js:6:2:6:54 | cp.exec ... temp")) |
 | tst_shell-command-injection-from-environment.js:9:2:9:58 | execa.s ... temp")) |
+| tst_shell-command-injection-from-environment.js:12:2:12:34 | execa.s ... + safe) |
 | uselesscat.js:16:1:16:29 | execSyn ... uinfo') |
 | uselesscat.js:18:1:18:26 | execSyn ... path}`) |
 | uselesscat.js:20:1:20:36 | execSyn ... wc -l') |

javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js

@@ -7,4 +7,7 @@ var cp = require('child_process'),
 
 	execa.shell('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
 	execa.shellSync('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
+
+	const safe = "\"" + path.join(__dirname, "temp") + "\"";
+	execa.shellSync('rm -rf ' + safe); // OK
 });