Merge pull request #10381 from erik-krogh/protoList

JS: recognize a list of bad strings as a sanitizer for `js/prototype-polluting-assignment`

Author: erik-krogh

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

@@ -124,7 +124,8 @@ class Configuration extends TaintTracking::Configuration {
     guard instanceof TypeofCheck or
     guard instanceof NumberGuard or
     guard instanceof EqualityCheck or
-    guard instanceof IncludesCheck
+    guard instanceof IncludesCheck or
+    guard instanceof DenyListInclusionGuard
   }
 }
 
@@ -275,3 +276,21 @@ private class IncludesCheck extends TaintTracking::LabeledSanitizerGuardNode, In
     outcome = this.getPolarity().booleanNot()
   }
 }
+
+/**
+ * A sanitizer guard that checks tests whether `x` is included in a list like `["__proto__"].includes(x)`.
+ */
+private class DenyListInclusionGuard extends TaintTracking::SanitizerGuardNode, InclusionTest {
+  DenyListInclusionGuard() {
+    this.getContainerNode()
+        .getALocalSource()
+        .(DataFlow::ArrayCreationNode)
+        .getAnElement()
+        .mayHaveStringValue("__proto__")
+  }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    e = this.getContainedNode().asExpr() and
+    outcome = super.getPolarity().booleanNot()
+  }
+}

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

@@ -165,6 +165,13 @@ nodes
 | tst.js:97:9:97:19 | req.query.x |
 | tst.js:97:9:97:19 | req.query.x |
 | tst.js:97:9:97:45 | req.que ... /g, '') |
+| tst.js:102:9:102:38 | taint |
+| tst.js:102:17:102:38 | String( ... y.data) |
+| tst.js:102:24:102:37 | req.query.data |
+| tst.js:102:24:102:37 | req.query.data |
+| tst.js:105:5:105:17 | object[taint] |
+| tst.js:105:5:105:17 | object[taint] |
+| tst.js:105:12:105:16 | taint |
 edges
 | lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
 | lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
@@ -318,6 +325,12 @@ edges
 | tst.js:97:9:97:19 | req.query.x | tst.js:97:9:97:45 | req.que ... /g, '') |
 | tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
 | tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
+| tst.js:102:9:102:38 | taint | tst.js:105:12:105:16 | taint |
+| tst.js:102:17:102:38 | String( ... y.data) | tst.js:102:9:102:38 | taint |
+| tst.js:102:24:102:37 | req.query.data | tst.js:102:17:102:38 | String( ... y.data) |
+| tst.js:102:24:102:37 | req.query.data | tst.js:102:17:102:38 | String( ... y.data) |
+| tst.js:105:12:105:16 | taint | tst.js:105:5:105:17 | object[taint] |
+| tst.js:105:12:105:16 | taint | tst.js:105:5:105:17 | object[taint] |
 #select
 | lib.js:6:7:6:9 | obj | lib.js:1:43:1:46 | path | lib.js:6:7:6:9 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:1:43:1:46 | path | library input |
 | 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 |
@@ -342,3 +355,4 @@ edges
 | tst.js:87:9:87:21 | object[taint] | tst.js:77:24:77:37 | req.query.data | tst.js:87:9:87:21 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:77:24:77:37 | req.query.data | user controlled input |
 | tst.js:94:5:94:37 | obj[req ... ', '')] | tst.js:94:9:94:19 | req.query.x | tst.js:94:5:94:37 | obj[req ... ', '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:94:9:94:19 | req.query.x | user controlled input |
 | tst.js:97:5:97:46 | obj[req ... g, '')] | tst.js:97:9:97:19 | req.query.x | tst.js:97:5:97:46 | obj[req ... g, '')] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:97:9:97:19 | req.query.x | user controlled input |
+| tst.js:105:5:105:17 | object[taint] | tst.js:102:24:102:37 | req.query.data | tst.js:105:5:105:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:102:24:102:37 | req.query.data | user controlled input |

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

@@ -97,3 +97,17 @@ app.get('/foo', (req, res) => {
     obj[req.query.x.replace(/__proto__/g, '')].x = 'foo'; // NOT OK - "__pr__proto__oto__"
     obj[req.query.x.replace('o', '0')].x = 'foo'; // OK
 });
+
+app.get('/bar', (req, res) => {
+    let taint = String(req.query.data);
+
+    let object = {};
+    object[taint][taint] = taint; // NOT OK
+    
+    const bad = ["__proto__", "constructor"];
+    if (bad.includes(taint)) {
+        return;
+    }
+
+    object[taint][taint] = taint; // OK
+});