recognize delete expresssions as a sink for js/prototype-polluting-assignment

Author: erik-krogh

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

@@ -49,6 +49,8 @@ module PrototypePollutingAssignment {
       this = any(DataFlow::PropWrite write).getBase()
       or
       this = any(ExtendCall c).getDestinationOperand()
+      or
+      this = any(DeleteExpr del).getOperand().flow().(DataFlow::PropRef).getBase()
     }
 
     override DataFlow::FlowLabel getAFlowLabel() { result instanceof ObjectPrototype }

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

@@ -70,6 +70,15 @@ nodes
 | lib.js:70:13:70:24 | obj[path[0]] |
 | lib.js:70:17:70:20 | path |
 | lib.js:70:17:70:23 | path[0] |
+| lib.js:83:7:83:25 | path |
+| lib.js:83:14:83:25 | arguments[1] |
+| lib.js:83:14:83:25 | arguments[1] |
+| lib.js:86:7:86:26 | proto |
+| lib.js:86:15:86:26 | obj[path[0]] |
+| lib.js:86:19:86:22 | path |
+| lib.js:86:19:86:25 | path[0] |
+| lib.js:87:10:87:14 | proto |
+| lib.js:87:10:87:14 | proto |
 | 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 |
@@ -175,6 +184,14 @@ edges
 | lib.js:70:17:70:20 | path | lib.js:70:17:70:23 | path[0] |
 | lib.js:70:17:70:23 | path[0] | lib.js:70:13:70:24 | obj[path[0]] |
 | lib.js:70:17:70:23 | path[0] | lib.js:70:13:70:24 | obj[path[0]] |
+| lib.js:83:7:83:25 | path | lib.js:86:19:86:22 | path |
+| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
+| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
+| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
+| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
+| lib.js:86:15:86:26 | obj[path[0]] | lib.js:86:7:86:26 | proto |
+| lib.js:86:19:86:22 | path | lib.js:86:19:86:25 | path[0] |
+| lib.js:86:19:86:25 | path[0] | lib.js:86:15:86:26 | 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 |
@@ -219,6 +236,7 @@ edges
 | 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 |
 | lib.js:70:13:70:24 | obj[path[0]] | lib.js:59:18:59:18 | s | lib.js:70:13:70:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:59:18:59:18 | s | library input |
+| lib.js:87:10:87:14 | proto | lib.js:83:14:83:25 | arguments[1] | lib.js:87:10:87:14 | proto | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:83:14:83:25 | arguments[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

@@ -69,6 +69,20 @@ class Foo {
     const value = this.value;
     return (obj[path[0]][path[1]] = value); // NOT OK
   }
+
+  safe() {
+    const obj = this.obj;
+    obj[path[0]] = this.value; // OK
+  }
 }
 
 module.exports.Foo = Foo;
+
+module.exports.delete = function() {
+  var obj = arguments[0];
+  var path = arguments[1];
+  delete obj[path[0]]; // OK
+  var prop = arguments[2];
+  var proto = obj[path[0]];
+  delete proto[prop]; // NOT
+}