add isSync predicate to SystemCommandExecution

javascript/ql/src/semmle/javascript/Concepts.qll

@@ -22,6 +22,9 @@ abstract class SystemCommandExecution extends DataFlow::Node {
    * to the command.
    */
   DataFlow::Node getArgumentList() { none() }
+
+  /** Holds if the command execution happens synchronously. */
+  abstract predicate isSync();
 }
 
 /**

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

@@ -623,6 +623,10 @@ module NodeJSLib {
       // all of the above methods take the argument list as their second argument
       result = getArgument(1)
     }
+
+    override predicate isSync() {
+      "Sync" = methodName.suffix(methodName.length() - 4)
+    }
   }
 
   /**

javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll

@@ -160,6 +160,8 @@ module ShellJS {
     override DataFlow::Node getACommandArgument() { result = getArgument(0) }
 
     override predicate isShellInterpreted(DataFlow::Node arg) { arg = getACommandArgument() }
+
+    override predicate isSync() {none ()}
   }
 
   /**

javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll

@@ -9,6 +9,7 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
   int cmdArg;
 
   boolean shell;
+  boolean sync;
 
   SystemCommandExecutors() {
     exists(string mod, DataFlow::SourceNode callee |
@@ -31,9 +32,11 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
         ) and
         cmdArg = 0
       |
-        callee = DataFlow::moduleMember(mod, method)
+        callee = DataFlow::moduleMember(mod, method) and
+        sync = getSync(method)
       )
       or
+      sync = false and
       (
         shell = false and
         (
@@ -65,4 +68,19 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
   override predicate isShellInterpreted(DataFlow::Node arg) {
     arg = getACommandArgument() and shell = true
   }
+
+  override predicate isSync() {
+    sync = true
+  }
+}
+
+/**
+ * Gets a boolean reflecting if the name ends with "sync" or "Sync". 
+ */ 
+bindingset[name]
+private boolean getSync(string name) {
+  if name.suffix(name.length() - 4) = "Sync" or name.suffix(name.length() - 4) = "sync" then
+    result = true
+  else
+    result = false
 }

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

@@ -18,6 +18,52 @@ readFile
 | uselesscat.js:94:1:94:43 | exec("c ... ut) {}) | fs.readFile("foo/bar", function(err, out) {...}) |
 | uselesscat.js:96:1:96:53 | exec("c ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) |
 | uselesscat.js:98:1:98:55 | exec("c ... h(out)) | fs.readFile("foo/bar", (err, out) => ...) |
+syncCommand
+| child_process-test.js:9:5:9:22 | cp.execSync("foo") |
+| child_process-test.js:11:5:11:26 | cp.exec ... ("foo") |
+| child_process-test.js:13:5:13:23 | cp.spawnSync("foo") |
+| child_process-test.js:18:5:18:20 | cp.execSync(cmd) |
+| child_process-test.js:20:5:20:24 | cp.execFileSync(cmd) |
+| child_process-test.js:22:5:22:21 | cp.spawnSync(cmd) |
+| command-line-parameter-command-injection.js:11:2:11:21 | cp.execSync(args[0]) |
+| command-line-parameter-command-injection.js:12:2:12:33 | cp.exec ... rgs[0]) |
+| command-line-parameter-command-injection.js:15:2:15:26 | cp.exec ... rgs[0]) |
+| command-line-parameter-command-injection.js:16:2:16:38 | cp.exec ... rgs[0]) |
+| command-line-parameter-command-injection.js:19:2:19:18 | cp.execSync(arg0) |
+| command-line-parameter-command-injection.js:20:2:20:30 | cp.exec ... + arg0) |
+| command-line-parameter-command-injection.js:26:2:26:51 | cp.exec ... tion"`) |
+| command-line-parameter-command-injection.js:27:2:27:58 | cp.exec ... tion"`) |
+| other.js:7:5:7:36 | require ... nc(cmd) |
+| other.js:9:5:9:35 | require ... nc(cmd) |
+| other.js:12:5:12:30 | require ... nc(cmd) |
+| third-party-command-injection.js:6:9:6:28 | cp.execSync(command) |
+| tst_shell-command-injection-from-environment.js:4:2:4:62 | cp.exec ... emp")]) |
+| tst_shell-command-injection-from-environment.js:5:2:5:54 | cp.exec ... temp")) |
+| uselesscat.js:16:1:16:29 | execSyn ... uinfo') |
+| uselesscat.js:18:1:18:26 | execSyn ... path}`) |
+| uselesscat.js:20:1:20:50 | child_p ... wc -l') |
+| uselesscat.js:22:1:22:38 | execSyn ... o/bar') |
+| uselesscat.js:24:1:24:35 | execSyn ... o/bar`) |
+| uselesscat.js:28:1:28:39 | execSyn ...  1000}) |
+| uselesscat.js:32:1:32:34 | execSyn ... path}`) |
+| uselesscat.js:34:1:34:54 | execSyn ... utf8'}) |
+| uselesscat.js:36:1:36:77 | execSyn ... utf8'}) |
+| uselesscat.js:38:1:38:43 | execSyn ... r/baz') |
+| uselesscat.js:40:1:40:40 | execSyn ... path}`) |
+| uselesscat.js:42:1:42:47 | execSyn ... File}`) |
+| uselesscat.js:44:1:44:34 | execSyn ... ' ')}`) |
+| uselesscat.js:48:1:48:41 | execSyn ... tool}`) |
+| uselesscat.js:51:9:51:31 | execSyn ... + file) |
+| uselesscat.js:54:1:54:39 | execSyn ...  + "'") |
+| uselesscat.js:74:1:74:60 | execFil ... utf8'}) |
+| uselesscat.js:76:1:76:39 | execFil ... xml' ]) |
+| uselesscat.js:79:1:79:46 | execFil ...   opts) |
+| uselesscat.js:82:1:82:90 | execFil ... String) |
+| uselesscat.js:84:1:84:115 | execFil ... ring'}) |
+| uselesscat.js:86:1:86:75 | execFil ... utf8'}) |
+| uselesscat.js:88:1:88:35 | execSyn ...  + foo) |
+| uselesscat.js:90:1:90:50 | execFil ... th}` ]) |
+| uselesscat.js:92:1:92:46 | execFil ... th}` ]) |
 #select
 | False negative | uselesscat.js:54:42:54:69 | // NOT  ... lagged] |
 | False negative | uselesscat.js:84:118:84:144 | // NOT  ... lagged] |

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

@@ -18,3 +18,7 @@ where
 select msg, comment
 
 query string readFile(UselessCat cat) { result = createReadFileCall(cat) }
+
+query SystemCommandExecution syncCommand() {
+  result.isSync()	
+}