Skip to content

Commit 9824311

Browse files
committed
recognize a list of bad strings as a sanitizer for js/prototype-polluting-assignment
1 parent 3384521 commit 9824311

File tree

3 files changed

+46
-0
lines changed

3 files changed

+46
-0
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,21 @@ private class IncludesCheck extends TaintTracking::LabeledSanitizerGuardNode, In
275275
outcome = this.getPolarity().booleanNot()
276276
}
277277
}
278+
279+
/**
280+
* A sanitizer guard that checks tests whether `x` is included in a list of strings.
281+
*/
282+
private class StringListCheck extends TaintTracking::SanitizerGuardNode, InclusionTest {
283+
StringListCheck() {
284+
this.getContainerNode()
285+
.getALocalSource()
286+
.(DataFlow::ArrayCreationNode)
287+
.getAnElement()
288+
.mayHaveStringValue("__proto__")
289+
}
290+
291+
override predicate sanitizes(boolean outcome, Expr e) {
292+
e = this.getContainedNode().asExpr() and
293+
outcome = super.getPolarity().booleanNot()
294+
}
295+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,13 @@ nodes
165165
| tst.js:97:9:97:19 | req.query.x |
166166
| tst.js:97:9:97:19 | req.query.x |
167167
| tst.js:97:9:97:45 | req.que ... /g, '') |
168+
| tst.js:102:9:102:38 | taint |
169+
| tst.js:102:17:102:38 | String( ... y.data) |
170+
| tst.js:102:24:102:37 | req.query.data |
171+
| tst.js:102:24:102:37 | req.query.data |
172+
| tst.js:105:5:105:17 | object[taint] |
173+
| tst.js:105:5:105:17 | object[taint] |
174+
| tst.js:105:12:105:16 | taint |
168175
edges
169176
| lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
170177
| lib.js:1:38:1:40 | obj | lib.js:6:7:6:9 | obj |
@@ -318,6 +325,12 @@ edges
318325
| tst.js:97:9:97:19 | req.query.x | tst.js:97:9:97:45 | req.que ... /g, '') |
319326
| tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
320327
| tst.js:97:9:97:45 | req.que ... /g, '') | tst.js:97:5:97:46 | obj[req ... g, '')] |
328+
| tst.js:102:9:102:38 | taint | tst.js:105:12:105:16 | taint |
329+
| tst.js:102:17:102:38 | String( ... y.data) | tst.js:102:9:102:38 | taint |
330+
| tst.js:102:24:102:37 | req.query.data | tst.js:102:17:102:38 | String( ... y.data) |
331+
| tst.js:102:24:102:37 | req.query.data | tst.js:102:17:102:38 | String( ... y.data) |
332+
| tst.js:105:12:105:16 | taint | tst.js:105:5:105:17 | object[taint] |
333+
| tst.js:105:12:105:16 | taint | tst.js:105:5:105:17 | object[taint] |
321334
#select
322335
| 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 |
323336
| 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
342355
| 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 |
343356
| 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 |
344357
| 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 |
358+
| 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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,17 @@ app.get('/foo', (req, res) => {
9797
obj[req.query.x.replace(/__proto__/g, '')].x = 'foo'; // NOT OK - "__pr__proto__oto__"
9898
obj[req.query.x.replace('o', '0')].x = 'foo'; // OK
9999
});
100+
101+
app.get('/bar', (req, res) => {
102+
let taint = String(req.query.data);
103+
104+
let object = {};
105+
object[taint][taint] = taint; // NOT OK
106+
107+
const bad = ["__proto__", "constructor"];
108+
if (bad.includes(taint)) {
109+
return;
110+
}
111+
112+
object[taint][taint] = taint; // OK
113+
});

0 commit comments

Comments
 (0)