Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* Added taint-steps for `Array.prototype.findLast`
* Added taint-steps for `Array.prototype.findLastIndex`
31 changes: 29 additions & 2 deletions javascript/ql/lib/semmle/javascript/Arrays.qll
Original file line number Diff line number Diff line change
Expand Up @@ -384,10 +384,10 @@ private module ArrayLibraries {
}

/**
* Gets a call to `Array.prototype.find` or a polyfill implementing the same functionality.
* Gets a call to `Array.prototype.find` or `Array.prototype.findLast` or a polyfill implementing the same functionality.
*/
DataFlow::CallNode arrayFindCall(DataFlow::Node array) {
result.(DataFlow::MethodCallNode).getMethodName() = "find" and
result.(DataFlow::MethodCallNode).getMethodName() in ["find", "findLast"] and
array = result.getReceiver()
or
result = DataFlow::moduleImport(["array.prototype.find", "array-find"]).getACall() and
Expand Down Expand Up @@ -483,4 +483,31 @@ private module ArrayLibraries {
)
}
}

/**
* Defines a data flow step that tracks the flow of data through callback functions in arrays.
*/
private class ArrayCallBackDataFlowStep extends PreCallGraphStep {
override predicate loadStep(DataFlow::Node obj, DataFlow::Node element, string prop) {
exists(DataFlow::MethodCallNode call |
call.getMethodName() = ["findLast", "find", "findLastIndex"] and
prop = arrayLikeElement() and
obj = call.getReceiver() and
element = call.getCallback(0).getParameter(0)
)
}
}

/**
* This step models the propagation of data from the array to the callback function's parameter.
*/
private class ArrayCallBackDataTaintStep extends TaintTracking::SharedTaintStep {
override predicate step(DataFlow::Node obj, DataFlow::Node element) {
exists(DataFlow::MethodCallNode call |
call.getMethodName() = ["findLast", "find", "findLastIndex"] and
obj = call.getReceiver() and
element = call.getCallback(0).getParameter(0)
)
}
}
}
2 changes: 1 addition & 1 deletion javascript/ql/src/Statements/UseOfReturnlessFunction.ql
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ predicate hasNonVoidCallbackMethod(string name) {
name =
[
"every", "filter", "find", "findIndex", "flatMap", "map", "reduce", "reduceRight", "some",
"sort"
"sort", "findLastIndex", "findLast"
]
}

Expand Down
6 changes: 6 additions & 0 deletions javascript/ql/test/library-tests/Arrays/DataFlow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
| arrays.js:2:16:2:23 | "source" | arrays.js:111:8:111:33 | arr.fin ... llback) |
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
Expand All @@ -25,3 +26,8 @@
| arrays.js:53:4:53:11 | "source" | arrays.js:54:10:54:18 | ary.pop() |
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |
| arrays.js:114:19:114:26 | "source" | arrays.js:115:50:115:53 | item |
| arrays.js:114:19:114:26 | "source" | arrays.js:116:10:116:16 | element |
| arrays.js:120:19:120:26 | "source" | arrays.js:121:46:121:49 | item |
| arrays.js:120:19:120:26 | "source" | arrays.js:122:10:122:16 | element |
| arrays.js:126:19:126:26 | "source" | arrays.js:127:55:127:58 | item |
5 changes: 4 additions & 1 deletion javascript/ql/test/library-tests/Arrays/DataFlow.ql
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import javascript
class ArrayFlowConfig extends DataFlow::Configuration {
ArrayFlowConfig() { this = "ArrayFlowConfig" }

override predicate isSource(DataFlow::Node source) { source.asExpr().getStringValue() = "source" }
override predicate isSource(DataFlow::Node source) {
source.asExpr().getStringValue() = "source" or
source.(DataFlow::CallNode).getCalleeName() = "source"
}

override predicate isSink(DataFlow::Node sink) {
sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
Expand Down
11 changes: 11 additions & 0 deletions javascript/ql/test/library-tests/Arrays/TaintFlow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
| arrays.js:2:16:2:23 | "source" | arrays.js:111:8:111:33 | arr.fin ... llback) |
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
Expand All @@ -29,3 +30,13 @@
| arrays.js:96:9:96:16 | "source" | arrays.js:96:8:96:36 | ["sourc ... => !!x) |
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |
| arrays.js:114:19:114:26 | "source" | arrays.js:115:50:115:53 | item |
| arrays.js:114:19:114:26 | "source" | arrays.js:116:10:116:16 | element |
| arrays.js:120:19:120:26 | "source" | arrays.js:121:46:121:49 | item |
| arrays.js:120:19:120:26 | "source" | arrays.js:122:10:122:16 | element |
| arrays.js:126:19:126:26 | "source" | arrays.js:127:55:127:58 | item |
| arrays.js:131:17:131:24 | source() | arrays.js:132:46:132:49 | item |
| arrays.js:131:17:131:24 | source() | arrays.js:133:10:133:17 | element1 |
| arrays.js:137:17:137:24 | source() | arrays.js:138:50:138:53 | item |
| arrays.js:137:17:137:24 | source() | arrays.js:139:10:139:17 | element1 |
| arrays.js:143:17:143:24 | source() | arrays.js:144:55:144:58 | item |
5 changes: 4 additions & 1 deletion javascript/ql/test/library-tests/Arrays/TaintFlow.ql
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import javascript
class ArrayTaintFlowConfig extends TaintTracking::Configuration {
ArrayTaintFlowConfig() { this = "ArrayTaintFlowConfig" }

override predicate isSource(DataFlow::Node source) { source.asExpr().getStringValue() = "source" }
override predicate isSource(DataFlow::Node source) {
source.asExpr().getStringValue() = "source" or
source.(DataFlow::CallNode).getCalleeName() = "source"
}

override predicate isSink(DataFlow::Node sink) {
sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
Expand Down
37 changes: 37 additions & 0 deletions javascript/ql/test/library-tests/Arrays/arrays.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,41 @@
var arr8_spread = [];
arr8_spread = arr8_spread.toSpliced(0, 0, ...arr);
sink(arr8_spread.pop()); // NOT OK

sink(arr.findLast(someCallback)); // NOT OK

{ // Test for findLast function
const list = ["source"];
const element = list.findLast((item) => sink(item)); // NOT OK
sink(element); // NOT OK
}

{ // Test for find function
const list = ["source"];
const element = list.find((item) => sink(item)); // NOT OK
sink(element); // NOT OK
}

{ // Test for findLastIndex function
const list = ["source"];
const element = list.findLastIndex((item) => sink(item)); // NOT OK
sink(element); // OK
}
{
const arr = source();
const element1 = arr.find((item) => sink(item)); // NOT OK
sink(element1); // NOT OK
}

{
const arr = source();
const element1 = arr.findLast((item) => sink(item)); // NOT OK
sink(element1); // NOT OK
}

{
const arr = source();
const element1 = arr.findLastIndex((item) => sink(item)); // NOT OK
sink(element1); // OK
}
});
Loading