Skip to content

Commit 671e7c6

Browse files
authored
Merge pull request github#3335 from asger-semmle/js/cached-chained-methods
Approved by esbena
2 parents d98e956 + cafdcfa commit 671e7c6

File tree

1 file changed

+63
-23
lines changed
  • javascript/ql/src/semmle/javascript/dataflow

1 file changed

+63
-23
lines changed

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

Lines changed: 63 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class SourceNode extends DataFlow::Node {
3939
* Holds if this node flows into `sink` in zero or more local (that is,
4040
* intra-procedural) steps.
4141
*/
42-
predicate flowsTo(DataFlow::Node sink) { hasLocalSource(sink, this) }
42+
predicate flowsTo(DataFlow::Node sink) { Cached::hasLocalSource(sink, this) }
4343

4444
/**
4545
* Holds if this node flows into `sink` in zero or more local (that is,
@@ -51,8 +51,7 @@ class SourceNode extends DataFlow::Node {
5151
* Gets a reference (read or write) of property `propName` on this node.
5252
*/
5353
DataFlow::PropRef getAPropertyReference(string propName) {
54-
result = getAPropertyReference() and
55-
result.getPropertyName() = propName
54+
Cached::namedPropRef(this, propName, result)
5655
}
5756

5857
/**
@@ -78,7 +77,11 @@ class SourceNode extends DataFlow::Node {
7877
/**
7978
* Gets a reference (read or write) of any property on this node.
8079
*/
81-
DataFlow::PropRef getAPropertyReference() { flowsTo(result.getBase()) }
80+
DataFlow::PropRef getAPropertyReference() {
81+
Cached::namedPropRef(this, _, result)
82+
or
83+
Cached::dynamicPropRef(this, result)
84+
}
8285

8386
/**
8487
* Gets a read of any property on this node.
@@ -113,11 +116,8 @@ class SourceNode extends DataFlow::Node {
113116
* that is, `o.m(...)` or `o[p](...)`.
114117
*/
115118
DataFlow::CallNode getAMethodCall(string methodName) {
116-
exists(PropAccess pacc |
117-
pacc = result.getCalleeNode().asExpr().getUnderlyingReference() and
118-
flowsToExpr(pacc.getBase()) and
119-
pacc.getPropertyName() = methodName
120-
)
119+
result = getAMemberInvocation(methodName) and
120+
Cached::isSyntacticMethodCall(result)
121121
}
122122

123123
/**
@@ -148,7 +148,7 @@ class SourceNode extends DataFlow::Node {
148148
/**
149149
* Gets an invocation (with our without `new`) of this node.
150150
*/
151-
DataFlow::InvokeNode getAnInvocation() { flowsTo(result.getCalleeNode()) }
151+
DataFlow::InvokeNode getAnInvocation() { Cached::invocation(this, result) }
152152

153153
/**
154154
* Gets a function call to this node.
@@ -192,21 +192,61 @@ class SourceNode extends DataFlow::Node {
192192
}
193193

194194
/**
195-
* Holds if `source` is a `SourceNode` that can reach `sink` via local flow steps.
196-
*
197-
* The slightly backwards parametering ordering is to force correct indexing.
195+
* Cached predicates used by the member predicates in `SourceNode`.
198196
*/
199197
cached
200-
private predicate hasLocalSource(DataFlow::Node sink, DataFlow::Node source) {
201-
// Declaring `source` to be a `SourceNode` currently causes a redundant check in the
202-
// recursive case, so instead we check it explicitly here.
203-
source = sink and
204-
source instanceof DataFlow::SourceNode
205-
or
206-
exists(DataFlow::Node mid |
207-
hasLocalSource(mid, source) and
208-
DataFlow::localFlowStep(mid, sink)
209-
)
198+
private module Cached {
199+
/**
200+
* Holds if `source` is a `SourceNode` that can reach `sink` via local flow steps.
201+
*
202+
* The slightly backwards parametering ordering is to force correct indexing.
203+
*/
204+
cached
205+
predicate hasLocalSource(DataFlow::Node sink, DataFlow::Node source) {
206+
// Declaring `source` to be a `SourceNode` currently causes a redundant check in the
207+
// recursive case, so instead we check it explicitly here.
208+
source = sink and
209+
source instanceof DataFlow::SourceNode
210+
or
211+
exists(DataFlow::Node mid |
212+
hasLocalSource(mid, source) and
213+
DataFlow::localFlowStep(mid, sink)
214+
)
215+
}
216+
217+
/**
218+
* Holds if `base` flows to the base of `ref` and `ref` has property name `prop`.
219+
*/
220+
cached
221+
predicate namedPropRef(DataFlow::SourceNode base, string prop, DataFlow::PropRef ref) {
222+
hasLocalSource(ref.getBase(), base) and
223+
ref.getPropertyName() = prop
224+
}
225+
226+
/**
227+
* Holds if `base` flows to the base of `ref` and `ref` has no known property name.
228+
*/
229+
cached
230+
predicate dynamicPropRef(DataFlow::SourceNode base, DataFlow::PropRef ref) {
231+
hasLocalSource(ref.getBase(), base) and
232+
not exists(ref.getPropertyName())
233+
}
234+
235+
/**
236+
* Holds if `func` flows to the callee of `invoke`.
237+
*/
238+
cached
239+
predicate invocation(DataFlow::SourceNode func, DataFlow::InvokeNode invoke) {
240+
hasLocalSource(invoke.getCalleeNode(), func)
241+
}
242+
243+
/**
244+
* Holds if `invoke` has the syntactic shape of a method call.
245+
*/
246+
cached
247+
predicate isSyntacticMethodCall(DataFlow::CallNode call) {
248+
call.getCalleeNode().asExpr().getUnderlyingReference() instanceof PropAccess
249+
}
210250
}
211251

212252
module SourceNode {

0 commit comments

Comments
 (0)