Skip to content

Commit 8ebbfb1

Browse files
authored
Merge pull request #19412 from asgerf/js/promise-all
JS: Better type-tracking through Promise.all()
2 parents c7e4853 + da5d799 commit 8ebbfb1

File tree

3 files changed

+58
-7
lines changed

3 files changed

+58
-7
lines changed

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

+4-4
Original file line numberDiff line numberDiff line change
@@ -850,10 +850,10 @@ module API {
850850
)
851851
or
852852
lbl = Label::promised() and
853-
PromiseFlow::storeStep(rhs, pred, Promises::valueProp())
853+
SharedTypeTrackingStep::storeStep(rhs, pred, Promises::valueProp())
854854
or
855855
lbl = Label::promisedError() and
856-
PromiseFlow::storeStep(rhs, pred, Promises::errorProp())
856+
SharedTypeTrackingStep::storeStep(rhs, pred, Promises::errorProp())
857857
or
858858
// The return-value of a getter G counts as a definition of property G
859859
// (Ordinary methods and properties are handled as PropWrite nodes)
@@ -1008,11 +1008,11 @@ module API {
10081008
propDesc = ""
10091009
)
10101010
or
1011-
PromiseFlow::loadStep(pred.getALocalUse(), ref, Promises::valueProp()) and
1011+
SharedTypeTrackingStep::loadStep(pred.getALocalUse(), ref, Promises::valueProp()) and
10121012
lbl = Label::promised() and
10131013
(propDesc = Promises::valueProp() or propDesc = "")
10141014
or
1015-
PromiseFlow::loadStep(pred.getALocalUse(), ref, Promises::errorProp()) and
1015+
SharedTypeTrackingStep::loadStep(pred.getALocalUse(), ref, Promises::errorProp()) and
10161016
lbl = Label::promisedError() and
10171017
(propDesc = Promises::errorProp() or propDesc = "")
10181018
}

javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll

+49-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
private import javascript
66
private import semmle.javascript.dataflow.FlowSummary
7+
private import semmle.javascript.dataflow.TypeTracking
78
private import FlowSummaryUtil
89

910
DataFlow::SourceNode promiseConstructorRef() {
@@ -211,12 +212,57 @@ private class PromiseReject extends SummarizedCallable {
211212
}
212213
}
213214

215+
/**
216+
* A call to `Promise.all()`.
217+
*/
218+
class PromiseAllCall extends DataFlow::CallNode {
219+
PromiseAllCall() { this = promiseConstructorRef().getAMemberCall("all") }
220+
221+
/** Gets the source of the input array */
222+
DataFlow::ArrayCreationNode getInputArray() { result = this.getArgument(0).getALocalSource() }
223+
224+
/** Gets the `n`th element of the input array */
225+
DataFlow::Node getNthInput(int n) { result = this.getInputArray().getElement(n) }
226+
227+
/** Gets a reference to the output array. */
228+
DataFlow::SourceNode getOutputArray() {
229+
exists(AwaitExpr await |
230+
this.flowsToExpr(await.getOperand()) and
231+
result = await.flow()
232+
)
233+
or
234+
result = this.getAMethodCall("then").getCallback(0).getParameter(0)
235+
}
236+
237+
/** Gets the `n`th output */
238+
DataFlow::SourceNode getNthOutput(int n) {
239+
exists(string prop |
240+
result = this.getOutputArray().getAPropertyRead(prop) and
241+
n = prop.toInt()
242+
)
243+
}
244+
}
245+
246+
/**
247+
* Helps type-tracking simple uses of `Promise.all()` such as `const [a, b] = await Promise.all([x, y])`.
248+
*
249+
* Due to limited access path depth, type tracking can't track things that are in a promise and an array
250+
* at once. This generates a step directly from the input array to the output array.
251+
*/
252+
private class PromiseAllStep extends SharedTypeTrackingStep {
253+
override predicate loadStep(DataFlow::Node node1, DataFlow::Node node2, string prop) {
254+
exists(PromiseAllCall call, int n |
255+
node1 = call.getNthInput(n) and
256+
node2 = call.getNthOutput(n) and
257+
prop = Promises::valueProp()
258+
)
259+
}
260+
}
261+
214262
private class PromiseAll extends SummarizedCallable {
215263
PromiseAll() { this = "Promise.all()" }
216264

217-
override DataFlow::InvokeNode getACallSimple() {
218-
result = promiseConstructorRef().getAMemberCall("all")
219-
}
265+
override DataFlow::InvokeNode getACallSimple() { result instanceof PromiseAllCall }
220266

221267
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
222268
preservesValue = true and
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Type information is now propagated more precisely through `Promise.all()` calls,
5+
leading to more resolved calls and more sources and sinks being detected.

0 commit comments

Comments
 (0)