diff --git a/javascript/ql/lib/change-notes/2024-11-20-ES2024-group-functions.md b/javascript/ql/lib/change-notes/2024-11-20-ES2024-group-functions.md new file mode 100644 index 000000000000..8511727f8e77 --- /dev/null +++ b/javascript/ql/lib/change-notes/2024-11-20-ES2024-group-functions.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added taint-steps for `Map.groupBy` and `Object.groupBy`. diff --git a/javascript/ql/lib/semmle/javascript/Collections.qll b/javascript/ql/lib/semmle/javascript/Collections.qll index a0e251554ff7..028c3abe4b3b 100644 --- a/javascript/ql/lib/semmle/javascript/Collections.qll +++ b/javascript/ql/lib/semmle/javascript/Collections.qll @@ -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) + ) + } + } + + /** + * 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() + ) + } + } } diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected index 3d4fd0b67f86..b6d5ab1e435c 100644 --- a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected +++ b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected @@ -248,8 +248,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 | diff --git a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected index 33a27661ecd1..3b89229b2d7f 100644 --- a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected +++ b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected @@ -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) | diff --git a/javascript/ql/test/library-tests/TaintTracking/tst.js b/javascript/ql/test/library-tests/TaintTracking/tst.js index 13b4ea48d8dd..8fbc5e525bd7 100644 --- a/javascript/ql/test/library-tests/TaintTracking/tst.js +++ b/javascript/ql/test/library-tests/TaintTracking/tst.js @@ -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