Skip to content

Commit 163285b

Browse files
authored
Merge pull request #2735 from asger-semmle/prototype-pollution-manual-dataflow
Approved by esbena
2 parents 67d7e83 + cf18bd7 commit 163285b

File tree

4 files changed

+594
-202
lines changed

4 files changed

+594
-202
lines changed

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import javascript
1515
import DataFlow
1616
import PathGraph
1717
import semmle.javascript.dataflow.InferredTypes
18+
import semmle.javascript.dataflow.internal.FlowSteps
1819

1920
/**
2021
* Gets a node that refers to an element of `array`, likely obtained
@@ -52,9 +53,12 @@ abstract class EnumeratedPropName extends DataFlow::Node {
5253
*
5354
* For example, gets `src[key]` in `for (var key in src) { src[key]; }`.
5455
*/
55-
PropRead getASourceProp() {
56-
result = AccessPath::getAnAliasedSourceNode(getSourceObject()).getAPropertyRead() and
57-
result.getPropertyNameExpr().flow().getImmediatePredecessor*() = this
56+
SourceNode getASourceProp() {
57+
exists(Node base, Node key |
58+
dynamicPropReadStep(base, key, result) and
59+
AccessPath::getAnAliasedSourceNode(getSourceObject()).flowsTo(base) and
60+
key.getImmediatePredecessor*() = this
61+
)
5862
}
5963
}
6064

@@ -102,7 +106,7 @@ class EntriesEnumeratedPropName extends EnumeratedPropName {
102106
result = entries.getArgument(0)
103107
}
104108

105-
override PropRead getASourceProp() {
109+
override SourceNode getASourceProp() {
106110
result = super.getASourceProp()
107111
or
108112
result = entry.getAPropertyRead("1")
@@ -133,6 +137,9 @@ class DynamicPropRead extends DataFlow::SourceNode, DataFlow::ValueNode {
133137
/** Gets the base of the dynamic read. */
134138
DataFlow::Node getBase() { result = astNode.getBase().flow() }
135139

140+
/** Gets the node holding the name of the property. */
141+
DataFlow::Node getPropertyNameNode() { result = astNode.getIndex().flow() }
142+
136143
/**
137144
* Holds if the value of this read was assigned to earlier in the same basic block.
138145
*
@@ -154,6 +161,77 @@ class DynamicPropRead extends DataFlow::SourceNode, DataFlow::ValueNode {
154161
}
155162
}
156163

164+
/**
165+
* Holds if `output` is the result of `base[key]`, either directly or through
166+
* one or more function calls, ignoring reads that can't access the prototype chain.
167+
*/
168+
predicate dynamicPropReadStep(Node base, Node key, SourceNode output) {
169+
exists(DynamicPropRead read |
170+
not read.hasDominatingAssignment() and
171+
base = read.getBase() and
172+
key = read.getPropertyNameNode() and
173+
output = read
174+
)
175+
or
176+
// Summarize functions returning a dynamic property read of two parameters, such as `function getProp(obj, prop) { return obj[prop]; }`.
177+
exists(CallNode call, Function callee, ParameterNode baseParam, ParameterNode keyParam, Node innerBase, Node innerKey, SourceNode innerOutput |
178+
dynamicPropReadStep(innerBase, innerKey, innerOutput) and
179+
baseParam.flowsTo(innerBase) and
180+
keyParam.flowsTo(innerKey) and
181+
innerOutput.flowsTo(callee.getAReturnedExpr().flow()) and
182+
call.getACallee() = callee and
183+
argumentPassingStep(call, base, callee, baseParam) and
184+
argumentPassingStep(call, key, callee, keyParam) and
185+
output = call
186+
)
187+
}
188+
189+
/**
190+
* Holds if `node` may flow from an enumerated prop name, possibly
191+
* into function calls (but not returns).
192+
*/
193+
predicate isEnumeratedPropName(Node node) {
194+
node instanceof EnumeratedPropName
195+
or
196+
exists(Node pred |
197+
isEnumeratedPropName(pred)
198+
|
199+
node = pred.getASuccessor()
200+
or
201+
argumentPassingStep(_, pred, _, node)
202+
or
203+
// Handle one level of callbacks
204+
exists(FunctionNode function, ParameterNode callback, int i |
205+
pred = callback.getAnInvocation().getArgument(i) and
206+
argumentPassingStep(_, function, _, callback) and
207+
node = function.getParameter(i)
208+
)
209+
)
210+
}
211+
212+
/**
213+
* Holds if `node` may refer to `Object.prototype` obtained through dynamic property
214+
* read of a property obtained through property enumeration.
215+
*/
216+
predicate isPotentiallyObjectPrototype(SourceNode node) {
217+
exists(Node base, Node key |
218+
dynamicPropReadStep(base, key, node) and
219+
isEnumeratedPropName(key) and
220+
221+
// Ignore cases where the properties of `base` are enumerated, to avoid FPs
222+
// where the key came from that enumeration (and thus will not return Object.prototype).
223+
// For example, `src[key]` in `for (let key in src) { ... src[key] ... }` will generally
224+
// not return Object.prototype because `key` is an enumerable property of `src`.
225+
not arePropertiesEnumerated(base.getALocalSource())
226+
)
227+
or
228+
exists(Node use |
229+
isPotentiallyObjectPrototype(use.getALocalSource())
230+
|
231+
argumentPassingStep(_, use, _, node)
232+
)
233+
}
234+
157235
/**
158236
* Holds if there is a dynamic property assignment of form `base[prop] = rhs`
159237
* which might act as the writing operation in a recursive merge function.
@@ -171,7 +249,12 @@ predicate dynamicPropWrite(DataFlow::Node base, DataFlow::Node prop, DataFlow::N
171249
prop = index.getPropertyNameExpr().flow() and
172250
rhs = write.getRhs().flow() and
173251
not exists(prop.getStringValue()) and
174-
not arePropertiesEnumerated(base.getALocalSource())
252+
not arePropertiesEnumerated(base.getALocalSource()) and
253+
254+
// Prune writes that are unlikely to modify Object.prototype.
255+
// This is mainly for performance, but may block certain results due to
256+
// not tracking out of function returns and into callbacks.
257+
isPotentiallyObjectPrototype(base.getALocalSource())
175258
)
176259
}
177260

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import javascript
2222
private import internal.CallGraphs
23+
private import internal.FlowSteps as FlowSteps
2324

2425
module DataFlow {
2526
cached
@@ -1477,6 +1478,8 @@ module DataFlow {
14771478
)
14781479
}
14791480

1481+
predicate argumentPassingStep = FlowSteps::argumentPassing/4;
1482+
14801483
/**
14811484
* Gets the data flow node representing the source of definition `def`, taking
14821485
* flow through IIFE calls into account.

0 commit comments

Comments
 (0)