Skip to content

Commit a0d3a6b

Browse files
committed
JS: Add withoutPropStep and model 'await' steps with it
1 parent 875776d commit a0d3a6b

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ module Promises {
189189
* Gets the pseudo-field used to describe rejected values in a promise.
190190
*/
191191
string errorProp() { result = "$PromiseRejectField$" }
192+
193+
/** A property set containing the pseudo-properites of a promise object. */
194+
class PromiseProps extends DataFlow::PropertySet {
195+
PromiseProps() { this = "PromiseProps" }
196+
197+
override string getAProperty() { result = [valueProp(), errorProp()] }
198+
}
192199
}
193200

194201
/**
@@ -274,6 +281,24 @@ private class PromiseStep extends PreCallGraphStep {
274281
}
275282
}
276283

284+
/**
285+
* A step from `p -> await p` for the case where `p` is not a promise.
286+
*
287+
* In this case, `await p` just returns `p` itself. We block flow of the promise-related
288+
* pseudo properties through this edge.
289+
*/
290+
private class RawAwaitStep extends DataFlow::SharedTypeTrackingStep {
291+
override predicate withoutPropStep(
292+
DataFlow::Node pred, DataFlow::Node succ, DataFlow::PropertySet props
293+
) {
294+
exists(AwaitExpr await |
295+
pred = await.getOperand().flow() and
296+
succ = await.flow() and
297+
props instanceof Promises::PromiseProps
298+
)
299+
}
300+
}
301+
277302
/**
278303
* This module defines how data-flow propagates into and out of a Promise.
279304
* The data-flow is based on pseudo-properties rather than tainting the Promise object (which is what `PromiseTaintStep` does).

javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ class TypeTracker extends TTypeTracker {
7070
step = LoadStep(prop) and result = MkTypeTracker(hasCall, "")
7171
or
7272
exists(string p | step = StoreStep(p) and prop = "" and result = MkTypeTracker(hasCall, p))
73+
or
74+
exists(PropertySet props |
75+
step = WithoutPropStep(props) and
76+
not prop = props.getAProperty() and
77+
result = this
78+
)
7379
}
7480

7581
/** Gets a textual representation of this summary. */
@@ -373,6 +379,26 @@ class SharedTypeTrackingStep extends Unit {
373379
) {
374380
none()
375381
}
382+
383+
/**
384+
* Holds if type-tracking should step from `pred` to `succ` but block flow of `props` through here.
385+
*
386+
* This can be seen as taking a copy of the value in `pred` but without the properties in `props`.
387+
*/
388+
predicate withoutPropStep(DataFlow::Node pred, DataFlow::Node succ, PropertySet props) { none() }
389+
}
390+
391+
/**
392+
* A representative for a set of property names.
393+
*
394+
* Currently this is used to denote a set of properties in `withoutPropStep`.
395+
*/
396+
abstract class PropertySet extends string {
397+
bindingset[this]
398+
PropertySet() { any() }
399+
400+
/** Gets a property contained in this property set. */
401+
abstract string getAProperty();
376402
}
377403

378404
/** Provides access to the steps contributed by subclasses of `SharedTypeTrackingStep`. */
@@ -413,6 +439,15 @@ module SharedTypeTrackingStep {
413439
) {
414440
any(SharedTypeTrackingStep s).loadStoreStep(pred, succ, loadProp, storeProp)
415441
}
442+
443+
/**
444+
* Holds if type-tracking should step from `pred` to `succ` but block flow of `prop` through here.
445+
*
446+
* This can be seen as taking a copy of the value in `pred` but without the properties in `props`.
447+
*/
448+
predicate withoutPropStep(DataFlow::Node pred, DataFlow::Node succ, PropertySet props) {
449+
any(SharedTypeTrackingStep s).withoutPropStep(pred, succ, props)
450+
}
416451
}
417452

418453
/**

javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ private module Cached {
4545
CopyStep(PropertyName prop) or
4646
LoadStoreStep(PropertyName fromProp, PropertyName toProp) {
4747
SharedTypeTrackingStep::loadStoreStep(_, _, fromProp, toProp)
48-
}
48+
} or
49+
WithoutPropStep(PropertySet props) { SharedTypeTrackingStep::withoutPropStep(_, _, props) }
4950
}
5051

5152
/**
@@ -110,6 +111,11 @@ private module Cached {
110111
summary = CopyStep(prop)
111112
)
112113
or
114+
exists(PropertySet props |
115+
SharedTypeTrackingStep::withoutPropStep(pred, succ, props) and
116+
summary = WithoutPropStep(props)
117+
)
118+
or
113119
exists(string fromProp, string toProp |
114120
SharedTypeTrackingStep::loadStoreStep(pred, succ, fromProp, toProp) and
115121
summary = LoadStoreStep(fromProp, toProp)
@@ -194,6 +200,8 @@ class StepSummary extends TStepSummary {
194200
or
195201
exists(string prop | this = CopyStep(prop) | result = "copy " + prop)
196202
or
203+
exists(string prop | this = WithoutPropStep(prop) | result = "without " + prop)
204+
or
197205
exists(string fromProp, string toProp | this = LoadStoreStep(fromProp, toProp) |
198206
result = "load " + fromProp + " and store to " + toProp
199207
)

0 commit comments

Comments
 (0)