Skip to content

Commit bd9405a

Browse files
asger-semmleasgerf
asger-semmle
authored and
asgerf
committed
JS: Guard against more FPs
1 parent 738123d commit bd9405a

File tree

3 files changed

+125
-1
lines changed

3 files changed

+125
-1
lines changed

‎javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,26 @@ class DynamicPropRead extends DataFlow::SourceNode, DataFlow::ValueNode {
144144

145145
/** Gets the base of the dynamic read. */
146146
DataFlow::Node getBase() { result = astNode.getBase().flow() }
147+
148+
/**
149+
* Holds if the value of this read was assigned to earlier in the same basic block.
150+
*
151+
* For example, this is true for `dst[x]` on line 2 below:
152+
* ```js
153+
* dst[x] = {};
154+
* dst[x][y] = src[y];
155+
* ```
156+
*/
157+
predicate hasDominatingAssignment() {
158+
exists(DataFlow::PropWrite write, BasicBlock bb, int i, int j, SsaVariable ssaVar |
159+
write = getBase().getALocalSource().getAPropertyWrite() and
160+
bb.getNode(i) = write.getWriteNode() and
161+
bb.getNode(j) = astNode and
162+
i < j and
163+
write.getPropertyNameExpr() = ssaVar.getAUse() and
164+
astNode.getIndex() = ssaVar.getAUse()
165+
)
166+
}
147167
}
148168

149169
/**
@@ -238,11 +258,13 @@ class PropNameTracking extends DataFlow::Configuration {
238258
// Step through `p -> x[p]`
239259
exists(PropRead read |
240260
pred = read.getPropertyNameExpr().flow() and
261+
not read.(DynamicPropRead).hasDominatingAssignment() and
241262
succ = read
242263
)
243264
or
244265
// Step through `x -> x[p]`
245266
exists(DynamicPropRead read |
267+
not read.hasDominatingAssignment() and
246268
pred = read.getBase() and
247269
succ = read
248270
)

‎javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.expected‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,42 @@ nodes
707707
| PrototypePollutionUtility/tests.js:280:24:280:31 | src[key] |
708708
| PrototypePollutionUtility/tests.js:280:28:280:30 | key |
709709
| PrototypePollutionUtility/tests.js:280:28:280:30 | key |
710+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src |
711+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src |
712+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path |
713+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path |
714+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key |
715+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key |
716+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key |
717+
| PrototypePollutionUtility/tests.js:289:40:289:42 | src |
718+
| PrototypePollutionUtility/tests.js:289:40:289:42 | src |
719+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
720+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
721+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
722+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
723+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
724+
| PrototypePollutionUtility/tests.js:289:44:289:46 | key |
725+
| PrototypePollutionUtility/tests.js:289:44:289:46 | key |
726+
| PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key |
727+
| PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key |
728+
| PrototypePollutionUtility/tests.js:289:76:289:78 | key |
729+
| PrototypePollutionUtility/tests.js:289:76:289:78 | key |
730+
| PrototypePollutionUtility/tests.js:292:24:292:27 | path |
731+
| PrototypePollutionUtility/tests.js:292:24:292:27 | path |
732+
| PrototypePollutionUtility/tests.js:292:24:292:27 | path |
733+
| PrototypePollutionUtility/tests.js:293:30:293:32 | key |
734+
| PrototypePollutionUtility/tests.js:293:30:293:32 | key |
735+
| PrototypePollutionUtility/tests.js:293:30:293:32 | key |
736+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src |
737+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src |
738+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
739+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
740+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
741+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
742+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
743+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
744+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key |
745+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key |
710746
| examples/PrototypePollutionUtility.js:1:16:1:18 | dst |
711747
| examples/PrototypePollutionUtility.js:1:16:1:18 | dst |
712748
| examples/PrototypePollutionUtility.js:1:21:1:23 | src |
@@ -1696,6 +1732,56 @@ edges
16961732
| PrototypePollutionUtility/tests.js:280:28:280:30 | key | PrototypePollutionUtility/tests.js:280:24:280:31 | src[key] |
16971733
| PrototypePollutionUtility/tests.js:280:28:280:30 | key | PrototypePollutionUtility/tests.js:280:24:280:31 | src[key] |
16981734
| PrototypePollutionUtility/tests.js:280:28:280:30 | key | PrototypePollutionUtility/tests.js:280:24:280:31 | src[key] |
1735+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src | PrototypePollutionUtility/tests.js:289:40:289:42 | src |
1736+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src | PrototypePollutionUtility/tests.js:289:40:289:42 | src |
1737+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src | PrototypePollutionUtility/tests.js:293:37:293:39 | src |
1738+
| PrototypePollutionUtility/tests.js:285:28:285:30 | src | PrototypePollutionUtility/tests.js:293:37:293:39 | src |
1739+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path | PrototypePollutionUtility/tests.js:292:24:292:27 | path |
1740+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path | PrototypePollutionUtility/tests.js:292:24:292:27 | path |
1741+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path | PrototypePollutionUtility/tests.js:292:24:292:27 | path |
1742+
| PrototypePollutionUtility/tests.js:285:33:285:36 | path | PrototypePollutionUtility/tests.js:292:24:292:27 | path |
1743+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:44:289:46 | key |
1744+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:44:289:46 | key |
1745+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:44:289:46 | key |
1746+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:44:289:46 | key |
1747+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:76:289:78 | key |
1748+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:76:289:78 | key |
1749+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:76:289:78 | key |
1750+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:289:76:289:78 | key |
1751+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1752+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1753+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1754+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1755+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1756+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1757+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:30:293:32 | key |
1758+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:41:293:43 | key |
1759+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:41:293:43 | key |
1760+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:41:293:43 | key |
1761+
| PrototypePollutionUtility/tests.js:286:14:286:16 | key | PrototypePollutionUtility/tests.js:293:41:293:43 | key |
1762+
| PrototypePollutionUtility/tests.js:289:40:289:42 | src | PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
1763+
| PrototypePollutionUtility/tests.js:289:40:289:42 | src | PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
1764+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1765+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1766+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1767+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1768+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1769+
| PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] | PrototypePollutionUtility/tests.js:285:28:285:30 | src |
1770+
| PrototypePollutionUtility/tests.js:289:44:289:46 | key | PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
1771+
| PrototypePollutionUtility/tests.js:289:44:289:46 | key | PrototypePollutionUtility/tests.js:289:40:289:47 | src[key] |
1772+
| PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key | PrototypePollutionUtility/tests.js:285:33:285:36 | path |
1773+
| PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key | PrototypePollutionUtility/tests.js:285:33:285:36 | path |
1774+
| PrototypePollutionUtility/tests.js:289:76:289:78 | key | PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key |
1775+
| PrototypePollutionUtility/tests.js:289:76:289:78 | key | PrototypePollutionUtility/tests.js:289:50:289:78 | path ? ... y : key |
1776+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1777+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1778+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1779+
| PrototypePollutionUtility/tests.js:293:37:293:39 | src | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1780+
| PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1781+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1782+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1783+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
1784+
| PrototypePollutionUtility/tests.js:293:41:293:43 | key | PrototypePollutionUtility/tests.js:293:37:293:44 | src[key] |
16991785
| examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst |
17001786
| examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:5:19:5:21 | dst |
17011787
| examples/PrototypePollutionUtility.js:1:16:1:18 | dst | examples/PrototypePollutionUtility.js:7:13:7:15 | dst |

‎javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility/tests.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,20 @@ function copyUsingReflect(dst, src) {
280280
dst[key] = src[key]; // NOT OK
281281
}
282282
});
283-
}
283+
}
284+
285+
function copyWithPath(dst, src, path) {
286+
for (let key in src) {
287+
if (src.hasOwnProperty(key)) {
288+
if (dst[key]) {
289+
copyWithPath(dst[key], src[key], path ? path + '.' + key : key);
290+
} else {
291+
let target = {};
292+
target[path] = {};
293+
target[path][key] = src[key]; // OK
294+
doSomething(target);
295+
}
296+
}
297+
}
298+
return dst;
299+
}

0 commit comments

Comments
 (0)