Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS: Added support for [Object, Map].groupBy ES2024 feature #18008

Merged
merged 10 commits into from
Nov 21, 2024
17 changes: 16 additions & 1 deletion javascript/ql/lib/semmle/javascript/Collections.qll
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,22 @@ private module CollectionDataFlow {
exists(DataFlow::MethodCallNode call |
call = DataFlow::globalVarRef(["Map", "Object"]).getAMemberCall("groupBy") and
pred = call.getArgument(0) and
succ = call
(succ = call.getCallback(1).getParameter(0) or succ = call.getALocalUse())
Napalys marked this conversation as resolved.
Show resolved Hide resolved
)
}
}

/**
* A step for handling data flow and taint tracking for the groupBy method on iterable objects.
* Ensures propagation of taint and data flow through the groupBy operation.
*/
private class GroupByDataFlowStep extends PreCallGraphStep {
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
exists(DataFlow::MethodCallNode call |
call = DataFlow::globalVarRef("Map").getAMemberCall("groupBy") and
pred = call.getArgument(0) and
succ = call and
prop = mapValueUnknownKey()
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,15 @@ typeInferenceMismatch
| tst.js:2:13:2:20 | source() | tst.js:70:10:70:18 | xReversed |
| tst.js:2:13:2:20 | source() | tst.js:72:10:72:31 | Map.gro ... z => z) |
| tst.js:2:13:2:20 | source() | tst.js:74:10:74:34 | Object. ... z => z) |
| tst.js:2:13:2:20 | source() | tst.js:78:55:78:58 | item |
| tst.js:2:13:2:20 | source() | tst.js:79:14:79:20 | grouped |
| tst.js:75:22:75:29 | source() | tst.js:75:10:75:52 | Map.gro ... (item)) |
| tst.js:75:22:75:29 | source() | tst.js:75:47:75:50 | item |
| tst.js:82:23:82:30 | source() | tst.js:83:58:83:61 | item |
| tst.js:82:23:82:30 | source() | tst.js:84:14:84:20 | grouped |
| tst.js:87:22:87:29 | source() | tst.js:90:14:90:25 | taintedValue |
| tst.js:93:22:93:29 | source() | tst.js:96:14:96:25 | taintedValue |
| tst.js:93:22:93:29 | source() | tst.js:97:14:97:26 | map.get(true) |
| xml.js:5:18:5:25 | source() | xml.js:8:14:8:17 | text |
| xml.js:12:17:12:24 | source() | xml.js:13:14:13:19 | result |
| xml.js:23:18:23:25 | source() | xml.js:20:14:20:17 | attr |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,5 @@
| thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
| tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |
| tst.js:2:13:2:20 | source() | tst.js:54:14:54:19 | unsafe |
| tst.js:93:22:93:29 | source() | tst.js:96:14:96:25 | taintedValue |
| tst.js:93:22:93:29 | source() | tst.js:97:14:97:26 | map.get(true) |
8 changes: 4 additions & 4 deletions javascript/ql/test/library-tests/TaintTracking/tst.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ function test() {
sink(Map.groupBy(source(), (item) => sink(item))); // NOT OK

{
const grouped = Map.groupBy(x, (item) => sink(item)); // NOT OK -- Should be tainted, but it is not
const grouped = Map.groupBy(x, (item) => sink(item)); // NOT OK
sink(grouped); // NOT OK
}
{
const list = [source()];
const grouped = Map.groupBy(list, (item) => sink(item)); // NOT OK -- Should be tainted, but it is not
const grouped = Map.groupBy(list, (item) => sink(item)); // NOT OK
sink(grouped); // NOT OK
}
{
Expand All @@ -93,7 +93,7 @@ function test() {
const data = source();
const map = Map.groupBy(data, item => item);
const taintedValue = map.get(true);
sink(taintedValue); // NOT OK -- Should be tainted, but it is not
sink(map.get(true)); // NOT OK -- Should be tainted, but it is not
sink(taintedValue); // NOT OK
sink(map.get(true)); // NOT OK
}
}