Skip to content

Commit b4b0569

Browse files
committed
two bugfixes
1 parent 0f0187d commit b4b0569

File tree

3 files changed

+20
-3
lines changed

3 files changed

+20
-3
lines changed

javascript/ql/src/semmle/javascript/Arrays.qll

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ private module ArrayDataFlow {
187187
*
188188
* And array elements can be stored into a resulting array using `map(...)`.
189189
* E.g. in `arr.map(e => foo)`, the resulting array (`arr.map(e => foo)`) will contain the element `foo`.
190+
*
191+
* And the second parameter in the callback is the array ifself, so there is a `loadStoreStep` from the array to that second parameter.
190192
*/
191193
private class ArrayIteration extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
192194
ArrayIteration() {
@@ -200,7 +202,7 @@ private module ArrayDataFlow {
200202
override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
201203
prop = arrayElement() and
202204
pred = this.getReceiver() and
203-
succ = getCallback(0).getParameter(any(int i | i = 0 or i = 2))
205+
succ = getCallback(0).getParameter(0)
204206
}
205207

206208
/**
@@ -212,6 +214,15 @@ private module ArrayDataFlow {
212214
pred = this.getCallback(0).getAReturn() and
213215
succ = this
214216
}
217+
218+
/**
219+
* Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
220+
*/
221+
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
222+
prop = arrayElement() and
223+
pred = this.getReceiver() and
224+
succ = getCallback(0).getParameter(2)
225+
}
215226
}
216227

217228
/**

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::SourceNode {
612612
DataFlow::ValueNode getElement(int i) {
613613
result = this.(ArrayLiteralNode).getElement(i) or
614614
result = this.(ArrayConstructorInvokeNode).getElement(i) or
615-
exists(DataFlow::CallNode call | call.getCalleeName() = "from" |
615+
exists(DataFlow::CallNode call | call.getCalleeName() = "from" and call = this |
616616
result = call.getArgument(i)
617617
)
618618
}
@@ -624,7 +624,7 @@ class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::SourceNode {
624624
int getSize() {
625625
result = this.(ArrayLiteralNode).getSize() or
626626
result = this.(ArrayConstructorInvokeNode).getSize() or
627-
exists(DataFlow::CallNode call | call.getCalleeName() = "from" |
627+
exists(DataFlow::CallNode call | call.getCalleeName() = "from" and call = this |
628628
result = call.getNumArgument()
629629
)
630630
}

javascript/ql/test/library-tests/Arrays/arrays.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,10 @@
3939
arr6[i] = arr5[i];
4040
}
4141
sink(arr6.pop()); // NOT OK
42+
43+
44+
Array.from("source").forEach((e, i, ary) => {
45+
sink(ary.pop()); // NOT OK
46+
sink(ary); // OK - its the array itself, not an element.
47+
})
4248
});

0 commit comments

Comments
 (0)