detect library input when the arguments object is converted to an array

Author: erik-krogh

javascript/ql/lib/semmle/javascript/PackageExports.qll

@@ -18,10 +18,28 @@ DataFlow::SourceNode getALibraryInputParameter() {
   |
     result = func.getParameter(any(int arg | arg >= bound))
     or
-    exists(DataFlow::PropRead read |
-      not read.getPropertyName() = "length" and
-      result = read and
-      read.getBase() = func.getFunction().getArgumentsVariable().getAnAccess().flow()
+    result = getAnArgumentsRead(func.getFunction())
+  )
+}
+
+private DataFlow::SourceNode getAnArgumentsRead(Function func) {
+  exists(DataFlow::PropRead read |
+    not read.getPropertyName() = "length" and
+    result = read
+  |
+    read.getBase() = func.getArgumentsVariable().getAnAccess().flow()
+    or
+    exists(DataFlow::MethodCallNode call |
+      call =
+        DataFlow::globalVarRef("Array")
+            .getAPropertyRead("prototype")
+            .getAPropertyRead("slice")
+            .getAMethodCall("call")
+      or
+      call = DataFlow::globalVarRef("Array").getAMethodCall("from")
+    |
+      call.getArgument(0) = func.getArgumentsVariable().getAnAccess().flow() and
+      call.flowsTo(read.getBase())
     )
   )
 }

javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected

@@ -38,6 +38,20 @@ nodes
 | lib.js:26:10:26:21 | obj[path[0]] |
 | lib.js:26:14:26:17 | path |
 | lib.js:26:14:26:20 | path[0] |
+| lib.js:32:7:32:20 | path |
+| lib.js:32:14:32:20 | args[1] |
+| lib.js:32:14:32:20 | args[1] |
+| lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:7:34:10 | path |
+| lib.js:34:7:34:13 | path[0] |
+| lib.js:40:7:40:20 | path |
+| lib.js:40:14:40:20 | args[1] |
+| lib.js:40:14:40:20 | args[1] |
+| lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:7:42:10 | path |
+| lib.js:42:7:42:13 | path[0] |
 | tst.js:5:9:5:38 | taint |
 | tst.js:5:17:5:38 | String( ... y.data) |
 | tst.js:5:24:5:37 | req.query.data |
@@ -115,6 +129,18 @@ edges
 | lib.js:26:14:26:17 | path | lib.js:26:14:26:20 | path[0] |
 | lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
 | lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
+| lib.js:32:7:32:20 | path | lib.js:34:7:34:10 | path |
+| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
+| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
+| lib.js:34:7:34:10 | path | lib.js:34:7:34:13 | path[0] |
+| lib.js:34:7:34:13 | path[0] | lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:34:7:34:13 | path[0] | lib.js:34:3:34:14 | obj[path[0]] |
+| lib.js:40:7:40:20 | path | lib.js:42:7:42:10 | path |
+| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
+| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
+| lib.js:42:7:42:10 | path | lib.js:42:7:42:13 | path[0] |
+| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
+| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
 | tst.js:5:9:5:38 | taint | tst.js:8:12:8:16 | taint |
 | tst.js:5:9:5:38 | taint | tst.js:9:12:9:16 | taint |
 | tst.js:5:9:5:38 | taint | tst.js:12:25:12:29 | taint |
@@ -156,6 +182,8 @@ edges
 | lib.js:15:3:15:14 | obj[path[0]] | lib.js:14:38:14:41 | path | lib.js:15:3:15:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:14:38:14:41 | path | library input |
 | lib.js:22:3:22:14 | obj[path[0]] | lib.js:20:14:20:25 | arguments[1] | lib.js:22:3:22:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:20:14:20:25 | arguments[1] | library input |
 | lib.js:26:10:26:21 | obj[path[0]] | lib.js:25:44:25:47 | path | lib.js:26:10:26:21 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:25:44:25:47 | path | library input |
+| lib.js:34:3:34:14 | obj[path[0]] | lib.js:32:14:32:20 | args[1] | lib.js:34:3:34:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:32:14:32:20 | args[1] | library input |
+| lib.js:42:3:42:14 | obj[path[0]] | lib.js:40:14:40:20 | args[1] | lib.js:42:3:42:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:40:14:40:20 | args[1] | library input |
 | tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
 | tst.js:9:5:9:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:9:5:9:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |
 | tst.js:14:5:14:32 | unsafeG ...  taint) | tst.js:5:24:5:37 | req.query.data | tst.js:14:5:14:32 | unsafeG ...  taint) | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |

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

@@ -25,3 +25,19 @@ module.exports.setWithArgs = function() {
 module.exports.usedInTest = function (obj, path, value) {
   return obj[path[0]][path[1]] = value; // NOT OK
 }
+
+module.exports.setWithArgs2 = function() {
+  const args = Array.prototype.slice.call(arguments);
+  var obj = args[0];
+  var path = args[1];
+  var value = args[2];
+  obj[path[0]][path[1]] = value; // NOT OK
+}
+
+module.exports.setWithArgs3 = function() {
+  const args = Array.from(arguments);
+  var obj = args[0];
+  var path = args[1];
+  var value = args[2];
+  obj[path[0]][path[1]] = value; // NOT OK
+}