Skip to content

Commit bd4947f

Browse files
erik-krogherik-krogh
authored
Merge pull request #10046 from erik-krogh/protoFunc
JS: generalize `BarrierGuardFunction`to work on function that have multiple parameters
2 parents 6e495ba + 14cfe2e commit bd4947f

File tree

3 files changed

+77
-5
lines changed

3 files changed

+77
-5
lines changed

‎javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,6 +2018,7 @@ private class BarrierGuardFunction extends Function {
20182018
BarrierGuardNode guard;
20192019
boolean guardOutcome;
20202020
string label;
2021+
int paramIndex;
20212022

20222023
BarrierGuardFunction() {
20232024
barrierGuardIsRelevant(guard) and
@@ -2041,19 +2042,18 @@ private class BarrierGuardFunction extends Function {
20412042
sanitizedParameter.flowsToExpr(e) and
20422043
barrierGuardBlocksExpr(guard, guardOutcome, e, label)
20432044
) and
2044-
getNumParameter() = 1 and
2045-
sanitizedParameter.getParameter() = getParameter(0)
2045+
sanitizedParameter.getParameter() = getParameter(paramIndex)
20462046
}
20472047

20482048
/**
20492049
* Holds if this function sanitizes argument `e` of call `call`, provided the call evaluates to `outcome`.
20502050
*/
20512051
predicate isBarrierCall(DataFlow::CallNode call, Expr e, boolean outcome, string lbl) {
20522052
exists(DataFlow::Node arg |
2053+
argumentPassing(pragma[only_bind_into](call), pragma[only_bind_into](arg),
2054+
pragma[only_bind_into](this), pragma[only_bind_into](sanitizedParameter)) and
20532055
arg.asExpr() = e and
2054-
arg = call.getArgument(0) and
2055-
call.getNumArgument() = 1 and
2056-
argumentPassing(call, arg, this, sanitizedParameter) and
2056+
arg = call.getArgument(paramIndex) and
20572057
outcome = guardOutcome and
20582058
lbl = label
20592059
)

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,6 +1528,31 @@ nodes
15281528
| tests.js:571:24:571:31 | src[key] |
15291529
| tests.js:571:28:571:30 | key |
15301530
| tests.js:571:28:571:30 | key |
1531+
| tests.js:576:30:576:32 | src |
1532+
| tests.js:576:30:576:32 | src |
1533+
| tests.js:577:14:577:16 | key |
1534+
| tests.js:577:14:577:16 | key |
1535+
| tests.js:577:14:577:16 | key |
1536+
| tests.js:580:38:580:40 | src |
1537+
| tests.js:580:38:580:40 | src |
1538+
| tests.js:580:38:580:45 | src[key] |
1539+
| tests.js:580:38:580:45 | src[key] |
1540+
| tests.js:580:38:580:45 | src[key] |
1541+
| tests.js:580:38:580:45 | src[key] |
1542+
| tests.js:580:38:580:45 | src[key] |
1543+
| tests.js:582:17:582:19 | key |
1544+
| tests.js:582:17:582:19 | key |
1545+
| tests.js:582:17:582:19 | key |
1546+
| tests.js:582:24:582:26 | src |
1547+
| tests.js:582:24:582:26 | src |
1548+
| tests.js:582:24:582:31 | src[key] |
1549+
| tests.js:582:24:582:31 | src[key] |
1550+
| tests.js:582:24:582:31 | src[key] |
1551+
| tests.js:582:24:582:31 | src[key] |
1552+
| tests.js:582:24:582:31 | src[key] |
1553+
| tests.js:582:24:582:31 | src[key] |
1554+
| tests.js:582:28:582:30 | key |
1555+
| tests.js:582:28:582:30 | key |
15311556
edges
15321557
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
15331558
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
@@ -3461,6 +3486,38 @@ edges
34613486
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
34623487
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
34633488
| tests.js:571:28:571:30 | key | tests.js:571:24:571:31 | src[key] |
3489+
| tests.js:576:30:576:32 | src | tests.js:580:38:580:40 | src |
3490+
| tests.js:576:30:576:32 | src | tests.js:580:38:580:40 | src |
3491+
| tests.js:576:30:576:32 | src | tests.js:582:24:582:26 | src |
3492+
| tests.js:576:30:576:32 | src | tests.js:582:24:582:26 | src |
3493+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3494+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3495+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3496+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3497+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3498+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3499+
| tests.js:577:14:577:16 | key | tests.js:582:17:582:19 | key |
3500+
| tests.js:577:14:577:16 | key | tests.js:582:28:582:30 | key |
3501+
| tests.js:577:14:577:16 | key | tests.js:582:28:582:30 | key |
3502+
| tests.js:577:14:577:16 | key | tests.js:582:28:582:30 | key |
3503+
| tests.js:577:14:577:16 | key | tests.js:582:28:582:30 | key |
3504+
| tests.js:580:38:580:40 | src | tests.js:580:38:580:45 | src[key] |
3505+
| tests.js:580:38:580:40 | src | tests.js:580:38:580:45 | src[key] |
3506+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3507+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3508+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3509+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3510+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3511+
| tests.js:580:38:580:45 | src[key] | tests.js:576:30:576:32 | src |
3512+
| tests.js:582:24:582:26 | src | tests.js:582:24:582:31 | src[key] |
3513+
| tests.js:582:24:582:26 | src | tests.js:582:24:582:31 | src[key] |
3514+
| tests.js:582:24:582:26 | src | tests.js:582:24:582:31 | src[key] |
3515+
| tests.js:582:24:582:26 | src | tests.js:582:24:582:31 | src[key] |
3516+
| tests.js:582:24:582:31 | src[key] | tests.js:582:24:582:31 | src[key] |
3517+
| tests.js:582:28:582:30 | key | tests.js:582:24:582:31 | src[key] |
3518+
| tests.js:582:28:582:30 | key | tests.js:582:24:582:31 | src[key] |
3519+
| tests.js:582:28:582:30 | key | tests.js:582:24:582:31 | src[key] |
3520+
| tests.js:582:28:582:30 | key | tests.js:582:24:582:31 | src[key] |
34643521
#select
34653522
| examples/PrototypePollutingFunction.js:7:13:7:15 | dst | examples/PrototypePollutingFunction.js:2:14:2:16 | key | examples/PrototypePollutingFunction.js:7:13:7:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | examples/PrototypePollutingFunction.js:2:21:2:23 | src | src | examples/PrototypePollutingFunction.js:7:13:7:15 | dst | dst |
34663523
| path-assignment.js:15:13:15:18 | target | path-assignment.js:8:19:8:25 | keys[i] | path-assignment.js:15:13:15:18 | target | The property chain $@ is recursively assigned to $@ without guarding against prototype pollution. | path-assignment.js:8:19:8:25 | keys[i] | here | path-assignment.js:15:13:15:18 | target | target |

‎javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingFunction/tests.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,3 +572,18 @@ function copyHasOwnProperty3(dst, src) {
572572
}
573573
}
574574
}
575+
576+
function indirectHasOwn(dst, src) {
577+
for (let key in src) {
578+
if (!src.hasOwnProperty(key)) continue;
579+
if (hasOwn(dst, key) && isObject(dst[key])) {
580+
indirectHasOwn(dst[key], src[key]);
581+
} else {
582+
dst[key] = src[key];
583+
}
584+
}
585+
}
586+
587+
function hasOwn(obj, key) {
588+
return obj.hasOwnProperty(key)
589+
}

0 commit comments

Comments
 (0)