recognize calls to Function where spread arguments are used

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/dataflow/CodeInjectionCustomizations.qll

@@ -170,7 +170,7 @@ module CodeInjection {
         exists(string callName | c = DataFlow::globalVarRef(callName).getAnInvocation() |
           callName = "eval" and index = 0
           or
-          callName = "Function"
+          callName = "Function" and index = -1
           or
           callName = "execScript" and index = 0
           or
@@ -185,14 +185,13 @@ module CodeInjection {
           callName = "setImmediate" and index = 0
         )
         or
-        exists(DataFlow::GlobalVarRefNode wasm, string methodName |
-          wasm.getName() = "WebAssembly" and c = wasm.getAMemberCall(methodName)
-        |
-          methodName = "compile" or
-          methodName = "compileStreaming"
-        )
+        c = DataFlow::globalVarRef("WebAssembly").getAMemberCall(["compile", "compileStreaming"]) and
+        index = -1
       |
         this = c.getArgument(index)
+        or
+        index = -1 and
+        this = c.getAnArgument()
       )
       or
       // node-serialize is not intended to be safe for untrusted inputs

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected

@@ -167,6 +167,8 @@ nodes
 | tst.js:33:14:33:19 | source |
 | tst.js:35:28:35:33 | source |
 | tst.js:35:28:35:33 | source |
+| tst.js:37:33:37:38 | source |
+| tst.js:37:33:37:38 | source |
 edges
 | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
 | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
@@ -278,6 +280,8 @@ edges
 | tst.js:29:9:29:82 | source | tst.js:33:14:33:19 | source |
 | tst.js:29:9:29:82 | source | tst.js:35:28:35:33 | source |
 | tst.js:29:9:29:82 | source | tst.js:35:28:35:33 | source |
+| tst.js:29:9:29:82 | source | tst.js:37:33:37:38 | source |
+| tst.js:29:9:29:82 | source | tst.js:37:33:37:38 | source |
 | tst.js:29:18:29:41 | documen ... .search | tst.js:29:18:29:82 | documen ... , "$1") |
 | tst.js:29:18:29:41 | documen ... .search | tst.js:29:18:29:82 | documen ... , "$1") |
 | tst.js:29:18:29:82 | documen ... , "$1") | tst.js:29:9:29:82 | source |
@@ -336,3 +340,4 @@ edges
 | tst.js:31:18:31:23 | source | tst.js:29:18:29:41 | documen ... .search | tst.js:31:18:31:23 | source | $@ flows to here and is interpreted as code. | tst.js:29:18:29:41 | documen ... .search | User-provided value |
 | tst.js:33:14:33:19 | source | tst.js:29:18:29:41 | documen ... .search | tst.js:33:14:33:19 | source | $@ flows to here and is interpreted as code. | tst.js:29:18:29:41 | documen ... .search | User-provided value |
 | tst.js:35:28:35:33 | source | tst.js:29:18:29:41 | documen ... .search | tst.js:35:28:35:33 | source | $@ flows to here and is interpreted as code. | tst.js:29:18:29:41 | documen ... .search | User-provided value |
+| tst.js:37:33:37:38 | source | tst.js:29:18:29:41 | documen ... .search | tst.js:37:33:37:38 | source | $@ flows to here and is interpreted as code. | tst.js:29:18:29:41 | documen ... .search | User-provided value |

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected

@@ -171,6 +171,8 @@ nodes
 | tst.js:33:14:33:19 | source |
 | tst.js:35:28:35:33 | source |
 | tst.js:35:28:35:33 | source |
+| tst.js:37:33:37:38 | source |
+| tst.js:37:33:37:38 | source |
 edges
 | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
 | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
@@ -286,6 +288,8 @@ edges
 | tst.js:29:9:29:82 | source | tst.js:33:14:33:19 | source |
 | tst.js:29:9:29:82 | source | tst.js:35:28:35:33 | source |
 | tst.js:29:9:29:82 | source | tst.js:35:28:35:33 | source |
+| tst.js:29:9:29:82 | source | tst.js:37:33:37:38 | source |
+| tst.js:29:9:29:82 | source | tst.js:37:33:37:38 | source |
 | tst.js:29:18:29:41 | documen ... .search | tst.js:29:18:29:82 | documen ... , "$1") |
 | tst.js:29:18:29:41 | documen ... .search | tst.js:29:18:29:82 | documen ... , "$1") |
 | tst.js:29:18:29:82 | documen ... , "$1") | tst.js:29:9:29:82 | source |

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/tst.js

@@ -34,5 +34,5 @@ $('<a>').attr("onclick", location.search.substring(1));
 
     new Function("a", "b", source); // NOT OK
 
-    new Function(...["a", "b"], source); // NOT OK - but not flagged [INCONSISTENCY]
+    new Function(...["a", "b"], source); // NOT OK
 })();