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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added taint-steps for `Map.groupBy` and `Object.groupBy`.
28 changes: 28 additions & 0 deletions javascript/ql/lib/semmle/javascript/Collections.qll
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,32 @@ private module CollectionDataFlow {
)
}
}

/**
* A step for a call to `groupBy` on an iterable object.
*/
private class GroupByTaintStep extends TaintTracking::SharedTaintStep {
override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(DataFlow::MethodCallNode call |
call = DataFlow::globalVarRef(["Map", "Object"]).getAMemberCall("groupBy") and
pred = call.getArgument(0) and
(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()
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,19 @@ typeInferenceMismatch
| tst.js:2:13:2:20 | source() | tst.js:66:10:66:16 | xSorted |
| tst.js:2:13:2:20 | source() | tst.js:68:10:68:23 | x.toReversed() |
| 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:17 | x.with() |
| tst.js:2:13:2:20 | source() | tst.js:74:10:74:14 | xWith |
| 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:2:13:2:20 | source() | tst.js:100:10:100:17 | x.with() |
| tst.js:2:13:2:20 | source() | tst.js:102:10:102:14 | xWith |
| 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) |
28 changes: 28 additions & 0 deletions javascript/ql/test/library-tests/TaintTracking/tst.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,34 @@ function test() {
const xReversed = x.toReversed();
sink(xReversed) // NOT OK

sink(Map.groupBy(x, z => z)); // NOT OK
sink(Custom.groupBy(x, z => z)); // OK
sink(Object.groupBy(x, z => z)); // NOT OK
sink(Map.groupBy(source(), (item) => sink(item))); // NOT OK

{
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
sink(grouped); // NOT OK
}
{
const data = source();
const result = Object.groupBy(data, item => item);
const taintedValue = result[notDefined()];
sink(taintedValue); // NOT OK
}
{
const data = source();
const map = Map.groupBy(data, item => item);
const taintedValue = map.get(true);
sink(taintedValue); // NOT OK
sink(map.get(true)); // NOT OK
}

sink(x.with()) // NOT OK
const xWith = x.with();
sink(xWith) // NOT OK
Expand Down