Skip to content

Commit fa377ac

Browse files
author
Stephan Brandauer
authored
Merge pull request #8946 from kaeluka/deepFillIn-FN
JS: fix a FN for prototype polluting function query
2 parents b74d1fd + 3f13a5e commit fa377ac

File tree

3 files changed

+198
-9
lines changed

3 files changed

+198
-9
lines changed

‎javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,9 @@ predicate isPrototypePollutingAssignment(Node base, Node prop, Node rhs, Node pr
518518
if propNameSource instanceof EnumeratedPropName
519519
then
520520
cfg.hasFlow(propNameSource, prop) and
521-
cfg.hasFlow(propNameSource.(EnumeratedPropName).getASourceProp(), rhs)
521+
cfg.hasFlow([propNameSource, AccessPath::getAnAliasedSourceNode(propNameSource)]
522+
.(EnumeratedPropName)
523+
.getASourceProp(), rhs)
522524
else (
523525
cfg.hasFlow(propNameSource.(SplitPropName).getAnAlias(), prop) and
524526
rhs.getALocalSource() instanceof ParameterNode
@@ -567,27 +569,27 @@ class ObjectCreateNullCall extends CallNode {
567569
}
568570

569571
from
570-
PropNameTracking cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Node prop, Node base,
571-
string msg, Node col1, Node col2
572+
PropNameTracking cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Node propNameSource,
573+
Node base, string msg, Node col1, Node col2
572574
where
573-
isPollutedPropName(prop) and
575+
isPollutedPropName(propNameSource) and
574576
cfg.hasFlowPath(source, sink) and
575-
isPrototypePollutingAssignment(base, _, _, prop) and
577+
isPrototypePollutingAssignment(base, _, _, propNameSource) and
576578
sink.getNode() = base and
577-
source.getNode() = prop and
579+
source.getNode() = propNameSource and
578580
(
579581
getANodeLeadingToBaseBase(base) instanceof ObjectLiteralNode
580582
or
581583
not getANodeLeadingToBaseBase(base) instanceof ObjectCreateNullCall
582584
) and
583585
// Generate different messages for deep merge and deep assign cases.
584-
if prop instanceof EnumeratedPropName
586+
if propNameSource instanceof EnumeratedPropName
585587
then (
586-
col1 = prop.(EnumeratedPropName).getSourceObject() and
588+
col1 = propNameSource.(EnumeratedPropName).getSourceObject() and
587589
col2 = base and
588590
msg = "Properties are copied from $@ to $@ without guarding against prototype pollution."
589591
) else (
590-
col1 = prop and
592+
col1 = propNameSource and
591593
col2 = base and
592594
msg =
593595
"The property chain $@ is recursively assigned to $@ without guarding against prototype pollution."

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

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,78 @@ nodes
14061406
| tests.js:529:24:529:31 | src[key] |
14071407
| tests.js:529:28:529:30 | key |
14081408
| tests.js:529:28:529:30 | key |
1409+
| tests.js:534:31:534:33 | obj |
1410+
| tests.js:534:31:534:33 | obj |
1411+
| tests.js:534:31:534:33 | obj |
1412+
| tests.js:534:31:534:33 | obj |
1413+
| tests.js:538:18:538:24 | keys[i] |
1414+
| tests.js:538:18:538:24 | keys[i] |
1415+
| tests.js:538:18:538:24 | keys[i] |
1416+
| tests.js:538:27:538:29 | obj |
1417+
| tests.js:538:27:538:29 | obj |
1418+
| tests.js:538:27:538:29 | obj |
1419+
| tests.js:538:27:538:29 | obj |
1420+
| tests.js:538:27:538:38 | obj[keys[i]] |
1421+
| tests.js:538:27:538:38 | obj[keys[i]] |
1422+
| tests.js:538:27:538:38 | obj[keys[i]] |
1423+
| tests.js:538:27:538:38 | obj[keys[i]] |
1424+
| tests.js:538:27:538:38 | obj[keys[i]] |
1425+
| tests.js:538:27:538:38 | obj[keys[i]] |
1426+
| tests.js:538:27:538:38 | obj[keys[i]] |
1427+
| tests.js:538:31:538:37 | keys[i] |
1428+
| tests.js:538:31:538:37 | keys[i] |
1429+
| tests.js:538:31:538:37 | keys[i] |
1430+
| tests.js:542:30:542:32 | dst |
1431+
| tests.js:542:30:542:32 | dst |
1432+
| tests.js:542:30:542:32 | dst |
1433+
| tests.js:542:30:542:32 | dst |
1434+
| tests.js:542:35:542:37 | src |
1435+
| tests.js:542:35:542:37 | src |
1436+
| tests.js:542:35:542:37 | src |
1437+
| tests.js:542:35:542:37 | src |
1438+
| tests.js:543:26:543:28 | src |
1439+
| tests.js:543:26:543:28 | src |
1440+
| tests.js:543:26:543:28 | src |
1441+
| tests.js:543:26:543:28 | src |
1442+
| tests.js:543:32:543:34 | key |
1443+
| tests.js:543:32:543:34 | key |
1444+
| tests.js:543:32:543:34 | key |
1445+
| tests.js:543:32:543:34 | key |
1446+
| tests.js:543:37:543:41 | value |
1447+
| tests.js:543:37:543:41 | value |
1448+
| tests.js:543:37:543:41 | value |
1449+
| tests.js:543:37:543:41 | value |
1450+
| tests.js:545:33:545:35 | dst |
1451+
| tests.js:545:33:545:35 | dst |
1452+
| tests.js:545:33:545:35 | dst |
1453+
| tests.js:545:33:545:35 | dst |
1454+
| tests.js:545:33:545:40 | dst[key] |
1455+
| tests.js:545:33:545:40 | dst[key] |
1456+
| tests.js:545:33:545:40 | dst[key] |
1457+
| tests.js:545:33:545:40 | dst[key] |
1458+
| tests.js:545:37:545:39 | key |
1459+
| tests.js:545:37:545:39 | key |
1460+
| tests.js:545:37:545:39 | key |
1461+
| tests.js:545:37:545:39 | key |
1462+
| tests.js:545:43:545:47 | value |
1463+
| tests.js:545:43:545:47 | value |
1464+
| tests.js:545:43:545:47 | value |
1465+
| tests.js:545:43:545:47 | value |
1466+
| tests.js:547:13:547:15 | dst |
1467+
| tests.js:547:13:547:15 | dst |
1468+
| tests.js:547:13:547:15 | dst |
1469+
| tests.js:547:13:547:15 | dst |
1470+
| tests.js:547:13:547:15 | dst |
1471+
| tests.js:547:17:547:19 | key |
1472+
| tests.js:547:17:547:19 | key |
1473+
| tests.js:547:17:547:19 | key |
1474+
| tests.js:547:17:547:19 | key |
1475+
| tests.js:547:17:547:19 | key |
1476+
| tests.js:547:24:547:28 | value |
1477+
| tests.js:547:24:547:28 | value |
1478+
| tests.js:547:24:547:28 | value |
1479+
| tests.js:547:24:547:28 | value |
1480+
| tests.js:547:24:547:28 | value |
14091481
edges
14101482
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
14111483
| examples/PrototypePollutingFunction.js:1:16:1:18 | dst | examples/PrototypePollutingFunction.js:5:19:5:21 | dst |
@@ -3179,6 +3251,102 @@ edges
31793251
| tests.js:529:28:529:30 | key | tests.js:529:24:529:31 | src[key] |
31803252
| tests.js:529:28:529:30 | key | tests.js:529:24:529:31 | src[key] |
31813253
| tests.js:529:28:529:30 | key | tests.js:529:24:529:31 | src[key] |
3254+
| tests.js:534:31:534:33 | obj | tests.js:538:27:538:29 | obj |
3255+
| tests.js:534:31:534:33 | obj | tests.js:538:27:538:29 | obj |
3256+
| tests.js:534:31:534:33 | obj | tests.js:538:27:538:29 | obj |
3257+
| tests.js:534:31:534:33 | obj | tests.js:538:27:538:29 | obj |
3258+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3259+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3260+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3261+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3262+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3263+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3264+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3265+
| tests.js:538:18:538:24 | keys[i] | tests.js:543:32:543:34 | key |
3266+
| tests.js:538:27:538:29 | obj | tests.js:538:27:538:38 | obj[keys[i]] |
3267+
| tests.js:538:27:538:29 | obj | tests.js:538:27:538:38 | obj[keys[i]] |
3268+
| tests.js:538:27:538:29 | obj | tests.js:538:27:538:38 | obj[keys[i]] |
3269+
| tests.js:538:27:538:29 | obj | tests.js:538:27:538:38 | obj[keys[i]] |
3270+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3271+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3272+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3273+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3274+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3275+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3276+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3277+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3278+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3279+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3280+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3281+
| tests.js:538:27:538:38 | obj[keys[i]] | tests.js:543:37:543:41 | value |
3282+
| tests.js:538:31:538:37 | keys[i] | tests.js:538:27:538:38 | obj[keys[i]] |
3283+
| tests.js:538:31:538:37 | keys[i] | tests.js:538:27:538:38 | obj[keys[i]] |
3284+
| tests.js:538:31:538:37 | keys[i] | tests.js:538:27:538:38 | obj[keys[i]] |
3285+
| tests.js:538:31:538:37 | keys[i] | tests.js:538:27:538:38 | obj[keys[i]] |
3286+
| tests.js:542:30:542:32 | dst | tests.js:545:33:545:35 | dst |
3287+
| tests.js:542:30:542:32 | dst | tests.js:545:33:545:35 | dst |
3288+
| tests.js:542:30:542:32 | dst | tests.js:545:33:545:35 | dst |
3289+
| tests.js:542:30:542:32 | dst | tests.js:545:33:545:35 | dst |
3290+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3291+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3292+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3293+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3294+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3295+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3296+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3297+
| tests.js:542:30:542:32 | dst | tests.js:547:13:547:15 | dst |
3298+
| tests.js:542:35:542:37 | src | tests.js:543:26:543:28 | src |
3299+
| tests.js:542:35:542:37 | src | tests.js:543:26:543:28 | src |
3300+
| tests.js:542:35:542:37 | src | tests.js:543:26:543:28 | src |
3301+
| tests.js:542:35:542:37 | src | tests.js:543:26:543:28 | src |
3302+
| tests.js:543:26:543:28 | src | tests.js:534:31:534:33 | obj |
3303+
| tests.js:543:26:543:28 | src | tests.js:534:31:534:33 | obj |
3304+
| tests.js:543:26:543:28 | src | tests.js:534:31:534:33 | obj |
3305+
| tests.js:543:26:543:28 | src | tests.js:534:31:534:33 | obj |
3306+
| tests.js:543:26:543:28 | src | tests.js:543:37:543:41 | value |
3307+
| tests.js:543:26:543:28 | src | tests.js:543:37:543:41 | value |
3308+
| tests.js:543:26:543:28 | src | tests.js:543:37:543:41 | value |
3309+
| tests.js:543:26:543:28 | src | tests.js:543:37:543:41 | value |
3310+
| tests.js:543:32:543:34 | key | tests.js:545:37:545:39 | key |
3311+
| tests.js:543:32:543:34 | key | tests.js:545:37:545:39 | key |
3312+
| tests.js:543:32:543:34 | key | tests.js:545:37:545:39 | key |
3313+
| tests.js:543:32:543:34 | key | tests.js:545:37:545:39 | key |
3314+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3315+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3316+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3317+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3318+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3319+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3320+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3321+
| tests.js:543:32:543:34 | key | tests.js:547:17:547:19 | key |
3322+
| tests.js:543:37:543:41 | value | tests.js:545:43:545:47 | value |
3323+
| tests.js:543:37:543:41 | value | tests.js:545:43:545:47 | value |
3324+
| tests.js:543:37:543:41 | value | tests.js:545:43:545:47 | value |
3325+
| tests.js:543:37:543:41 | value | tests.js:545:43:545:47 | value |
3326+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3327+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3328+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3329+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3330+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3331+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3332+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3333+
| tests.js:543:37:543:41 | value | tests.js:547:24:547:28 | value |
3334+
| tests.js:545:33:545:35 | dst | tests.js:545:33:545:40 | dst[key] |
3335+
| tests.js:545:33:545:35 | dst | tests.js:545:33:545:40 | dst[key] |
3336+
| tests.js:545:33:545:35 | dst | tests.js:545:33:545:40 | dst[key] |
3337+
| tests.js:545:33:545:35 | dst | tests.js:545:33:545:40 | dst[key] |
3338+
| tests.js:545:33:545:40 | dst[key] | tests.js:542:30:542:32 | dst |
3339+
| tests.js:545:33:545:40 | dst[key] | tests.js:542:30:542:32 | dst |
3340+
| tests.js:545:33:545:40 | dst[key] | tests.js:542:30:542:32 | dst |
3341+
| tests.js:545:33:545:40 | dst[key] | tests.js:542:30:542:32 | dst |
3342+
| tests.js:545:37:545:39 | key | tests.js:545:33:545:40 | dst[key] |
3343+
| tests.js:545:37:545:39 | key | tests.js:545:33:545:40 | dst[key] |
3344+
| tests.js:545:37:545:39 | key | tests.js:545:33:545:40 | dst[key] |
3345+
| tests.js:545:37:545:39 | key | tests.js:545:33:545:40 | dst[key] |
3346+
| tests.js:545:43:545:47 | value | tests.js:542:35:542:37 | src |
3347+
| tests.js:545:43:545:47 | value | tests.js:542:35:542:37 | src |
3348+
| tests.js:545:43:545:47 | value | tests.js:542:35:542:37 | src |
3349+
| tests.js:545:43:545:47 | value | tests.js:542:35:542:37 | src |
31823350
#select
31833351
| 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 |
31843352
| 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 |
@@ -3209,3 +3377,4 @@ edges
32093377
| tests.js:489:13:489:15 | dst | tests.js:484:14:484:16 | key | tests.js:489:13:489:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:484:21:484:23 | src | src | tests.js:489:13:489:15 | dst | dst |
32103378
| tests.js:517:35:517:37 | dst | tests.js:511:19:511:25 | keys[i] | tests.js:517:35:517:37 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:509:28:509:30 | src | src | tests.js:517:35:517:37 | dst | dst |
32113379
| tests.js:529:13:529:15 | dst | tests.js:525:14:525:16 | key | tests.js:529:13:529:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:525:21:525:23 | src | src | tests.js:529:13:529:15 | dst | dst |
3380+
| tests.js:547:13:547:15 | dst | tests.js:538:18:538:24 | keys[i] | tests.js:547:13:547:15 | dst | Properties are copied from $@ to $@ without guarding against prototype pollution. | tests.js:535:30:535:32 | obj | obj | tests.js:547:13:547:15 | dst | dst |

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,3 +530,21 @@ function copyUsingForInAndRest(...args) {
530530
}
531531
}
532532
}
533+
534+
function forEachPropNoTempVar(obj, callback) {
535+
const keys = Object.keys(obj)
536+
const len = keys.length
537+
for (let i = 0; i < len; i++) {
538+
callback(keys[i], obj[keys[i]])
539+
}
540+
}
541+
542+
function mergeUsingCallback3(dst, src) {
543+
forEachPropNoTempVar(src, (key, value) => {
544+
if (dst[key]) {
545+
mergeUsingCallback3(dst[key], value);
546+
} else {
547+
dst[key] = value; // NOT OK
548+
}
549+
});
550+
}

0 commit comments

Comments
 (0)