Skip to content

Commit 61e0086

Browse files
authored
Merge pull request #18008 from Napalys/napalys/ES2024-group-functions
JS: Added support for [Object, Map].groupBy ES2024 feature
2 parents edb9b47 + 7ee0a7b commit 61e0086

File tree

5 files changed

+75
-2
lines changed

5 files changed

+75
-2
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added taint-steps for `Map.groupBy` and `Object.groupBy`.

javascript/ql/lib/semmle/javascript/Collections.qll

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,32 @@ private module CollectionDataFlow {
151151
)
152152
}
153153
}
154+
155+
/**
156+
* A step for a call to `groupBy` on an iterable object.
157+
*/
158+
private class GroupByTaintStep extends TaintTracking::SharedTaintStep {
159+
override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
160+
exists(DataFlow::MethodCallNode call |
161+
call = DataFlow::globalVarRef(["Map", "Object"]).getAMemberCall("groupBy") and
162+
pred = call.getArgument(0) and
163+
(succ = call.getCallback(1).getParameter(0) or succ = call)
164+
)
165+
}
166+
}
167+
168+
/**
169+
* A step for handling data flow and taint tracking for the groupBy method on iterable objects.
170+
* Ensures propagation of taint and data flow through the groupBy operation.
171+
*/
172+
private class GroupByDataFlowStep extends PreCallGraphStep {
173+
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
174+
exists(DataFlow::MethodCallNode call |
175+
call = DataFlow::globalVarRef("Map").getAMemberCall("groupBy") and
176+
pred = call.getArgument(0) and
177+
succ = call and
178+
prop = mapValueUnknownKey()
179+
)
180+
}
181+
}
154182
}

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,19 @@ typeInferenceMismatch
248248
| tst.js:2:13:2:20 | source() | tst.js:66:10:66:16 | xSorted |
249249
| tst.js:2:13:2:20 | source() | tst.js:68:10:68:23 | x.toReversed() |
250250
| tst.js:2:13:2:20 | source() | tst.js:70:10:70:18 | xReversed |
251-
| tst.js:2:13:2:20 | source() | tst.js:72:10:72:17 | x.with() |
252-
| tst.js:2:13:2:20 | source() | tst.js:74:10:74:14 | xWith |
251+
| tst.js:2:13:2:20 | source() | tst.js:72:10:72:31 | Map.gro ... z => z) |
252+
| tst.js:2:13:2:20 | source() | tst.js:74:10:74:34 | Object. ... z => z) |
253+
| tst.js:2:13:2:20 | source() | tst.js:78:55:78:58 | item |
254+
| tst.js:2:13:2:20 | source() | tst.js:79:14:79:20 | grouped |
255+
| tst.js:2:13:2:20 | source() | tst.js:100:10:100:17 | x.with() |
256+
| tst.js:2:13:2:20 | source() | tst.js:102:10:102:14 | xWith |
257+
| tst.js:75:22:75:29 | source() | tst.js:75:10:75:52 | Map.gro ... (item)) |
258+
| tst.js:75:22:75:29 | source() | tst.js:75:47:75:50 | item |
259+
| tst.js:82:23:82:30 | source() | tst.js:83:58:83:61 | item |
260+
| tst.js:82:23:82:30 | source() | tst.js:84:14:84:20 | grouped |
261+
| tst.js:87:22:87:29 | source() | tst.js:90:14:90:25 | taintedValue |
262+
| tst.js:93:22:93:29 | source() | tst.js:96:14:96:25 | taintedValue |
263+
| tst.js:93:22:93:29 | source() | tst.js:97:14:97:26 | map.get(true) |
253264
| xml.js:5:18:5:25 | source() | xml.js:8:14:8:17 | text |
254265
| xml.js:12:17:12:24 | source() | xml.js:13:14:13:19 | result |
255266
| xml.js:23:18:23:25 | source() | xml.js:20:14:20:17 | attr |

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,5 @@
112112
| thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
113113
| tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |
114114
| tst.js:2:13:2:20 | source() | tst.js:54:14:54:19 | unsafe |
115+
| tst.js:93:22:93:29 | source() | tst.js:96:14:96:25 | taintedValue |
116+
| tst.js:93:22:93:29 | source() | tst.js:97:14:97:26 | map.get(true) |

javascript/ql/test/library-tests/TaintTracking/tst.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,34 @@ function test() {
6969
const xReversed = x.toReversed();
7070
sink(xReversed) // NOT OK
7171

72+
sink(Map.groupBy(x, z => z)); // NOT OK
73+
sink(Custom.groupBy(x, z => z)); // OK
74+
sink(Object.groupBy(x, z => z)); // NOT OK
75+
sink(Map.groupBy(source(), (item) => sink(item))); // NOT OK
76+
77+
{
78+
const grouped = Map.groupBy(x, (item) => sink(item)); // NOT OK
79+
sink(grouped); // NOT OK
80+
}
81+
{
82+
const list = [source()];
83+
const grouped = Map.groupBy(list, (item) => sink(item)); // NOT OK
84+
sink(grouped); // NOT OK
85+
}
86+
{
87+
const data = source();
88+
const result = Object.groupBy(data, item => item);
89+
const taintedValue = result[notDefined()];
90+
sink(taintedValue); // NOT OK
91+
}
92+
{
93+
const data = source();
94+
const map = Map.groupBy(data, item => item);
95+
const taintedValue = map.get(true);
96+
sink(taintedValue); // NOT OK
97+
sink(map.get(true)); // NOT OK
98+
}
99+
72100
sink(x.with()) // NOT OK
73101
const xWith = x.with();
74102
sink(xWith) // NOT OK

0 commit comments

Comments
 (0)