Skip to content

Commit 972c4d6

Browse files
committed
JS: Add PrototypePollutingAssignment
1 parent ef52c46 commit 972c4d6

8 files changed

Lines changed: 345 additions & 0 deletions

File tree

javascript/config/suites/javascript/security

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
+ semmlecode-javascript-queries/Security/CWE-352/MissingCsrfMiddleware.ql: /Security/CWE/CWE-352
4040
+ semmlecode-javascript-queries/Security/CWE-400/PrototypePollution.ql: /Security/CWE/CWE-400
4141
+ semmlecode-javascript-queries/Security/CWE-400/PrototypePollutionUtility.ql: /Security/CWE/CWE-400
42+
+ semmlecode-javascript-queries/Security/CWE-471/PrototypePollutingAssignment.ql: /Security/CWE/CWE-471
4243
+ semmlecode-javascript-queries/Security/CWE-400/RemotePropertyInjection.ql: /Security/CWE/CWE-400
4344
+ semmlecode-javascript-queries/Security/CWE-502/UnsafeDeserialization.ql: /Security/CWE/CWE-502
4445
+ semmlecode-javascript-queries/Security/CWE-506/HardcodedDataInterpretedAsCode.ql: /Security/CWE/CWE-506
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Most JavaScript objects inherit the properties of the built-in <code>Object.prototype</code> object.
9+
Prototype pollution is a type of vulnerability in which an attacker is able to modify <code>Object.prototype</code>.
10+
Since most objects inherit from the compromised <code>Object.prototype</code>, the attacker can use this
11+
to tamper with the application logic, and often escalate to remote code execution or cross-site scripting.
12+
</p>
13+
14+
<p>
15+
One way to cause prototype pollution is by modifying an object obtained via a user-controlled property name.
16+
Most objects have a special <code>__proto__</code> property that refers to <code>Object.prototype</code>.
17+
An attacker can abuse this special property to trick the application into performing unintended modifications
18+
of <code>Object.prototype</code>.
19+
</p>
20+
</overview>
21+
22+
<recommendation>
23+
<p>
24+
Use an associative data structure that is resilient to untrusted key values, such as a <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map">Map</a>.
25+
In some cases, a prototype-less object created with <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create">Object.create(null)</a>
26+
may be preferrable.
27+
</p>
28+
<p>
29+
Alternatively, restrict the computed property name so it can't clash with a built-in property, either by
30+
prefixing it with a constant string, or by rejecting inputs that don't conform to the expected format.
31+
</p>
32+
</recommendation>
33+
34+
<example>
35+
<p>
36+
In the example below, the untrusted value <code>req.params.id</code> is used as the property name
37+
<code>req.session.todos[id]</code>. If a malicious user passes in the ID value <code>__proto__</code>,
38+
the variable <code>todo</code> will then refer to <code>Object.prototype</code>.
39+
Finally, the modification of <code>todo</code> then allows the attacker to inject arbitrary properties
40+
onto <code>Object.prototype</code>.
41+
</p>
42+
43+
<sample src="examples/PrototypePollutingAssignment.js"/>
44+
45+
<p>
46+
One way to fix this is to use <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map">Map</a> objects to associate key/value pairs
47+
instead of regular objects, as shown below:
48+
</p>
49+
50+
<sample src="examples/PrototypePollutingAssignmentFixed.js"/>
51+
52+
</example>
53+
54+
<references>
55+
<li>MDN:
56+
<a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto">Object.prototype.__proto__</a>
57+
</li>
58+
<li>MDN:
59+
<a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map">Map</a>
60+
</li>
61+
</references>
62+
</qhelp>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Prototype-polluting assignment
3+
* @description Modifying an object obtained via a user-controlled property name may
4+
* lead to accidental modification of the built-in Object.prototype,
5+
* and possibly escalate to remote code execution or cross-site scripting.
6+
* @kind path-problem
7+
* @problem.severity warning
8+
* @precision high
9+
* @id js/prototype-polluting-assignment
10+
* @tags security
11+
* external/cwe/cwe-250
12+
* external/cwe/cwe-471
13+
*/
14+
15+
import javascript
16+
import semmle.javascript.security.dataflow.PrototypePollutingAssignment::PrototypePollutingAssignment
17+
import DataFlow::PathGraph
18+
19+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
20+
where cfg.hasFlowPath(source, sink)
21+
select sink, source, sink,
22+
"This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@.",
23+
source.getNode(), "here"
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
let express = require('express');
2+
3+
express.put('/todos/:id', (req, res) => {
4+
let id = req.params.id;
5+
let items = req.session.todos[id];
6+
if (!items) {
7+
items = req.session.todos[id] = {};
8+
}
9+
items[req.query.name] = req.query.text;
10+
res.end(200);
11+
});
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
let express = require('express');
2+
3+
express.put('/todos/:id', (req, res) => {
4+
let id = req.params.id;
5+
let items = req.session.todos.get(id);
6+
if (!items) {
7+
items = new Map();
8+
req.sessions.todos.set(id, items);
9+
}
10+
items.set(req.query.name, req.query.text);
11+
res.end(200);
12+
});

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,7 @@ private predicate summarizedHigherOrderCall(
12811281
DataFlow::Node innerArg, DataFlow::SourceNode cbParm, PathSummary oldSummary
12821282
|
12831283
reachableFromInput(f, outer, arg, innerArg, cfg, oldSummary) and
1284+
not arg = DataFlow::capturedVariableNode(_) and // Only track actual parameter flow
12841285
argumentPassing(outer, cb, f, cbParm) and
12851286
innerArg = inner.getArgument(j)
12861287
|
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about
3+
* prototype-polluting assignments.
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `PrototypePollutingAssignment::Configuration` is needed, otherwise
7+
* `PrototypePollutingAssignmentCustomizations` should be imported instead.
8+
*/
9+
10+
private import javascript
11+
private import semmle.javascript.DynamicPropertyAccess
12+
13+
module PrototypePollutingAssignment {
14+
private import PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment
15+
16+
// Materialize flow labels
17+
private class ConcreteObjectPrototype extends ObjectPrototype { }
18+
19+
/** A taint-tracking configuration for reasoning about prototype-polluting assignments. */
20+
class Configuration extends TaintTracking::Configuration {
21+
Configuration() { this = "PrototypePollutingAssignment" }
22+
23+
override predicate isSource(DataFlow::Node node) { node instanceof Source }
24+
25+
override predicate isSink(DataFlow::Node node, DataFlow::FlowLabel lbl) {
26+
node.(Sink).getAFlowLabel() = lbl
27+
}
28+
29+
override predicate isSanitizer(DataFlow::Node node) {
30+
node instanceof Sanitizer
31+
or
32+
// Concatenating with a string will in practice prevent the string `__proto__` from arising.
33+
node instanceof StringOps::ConcatenationRoot
34+
}
35+
36+
override predicate isAdditionalFlowStep(
37+
DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl,
38+
DataFlow::FlowLabel outlbl
39+
) {
40+
// Step from x -> obj[x] while switching to the ObjectPrototype label
41+
// (If `x` can have the value `__proto__` then the result can be Object.prototype)
42+
exists(DataFlow::PropRead read |
43+
pred = read.getPropertyNameExpr().flow() and
44+
succ = read and
45+
inlbl.isTaint() and
46+
outlbl instanceof ObjectPrototype and
47+
// Exclude cases where the property name came from a property enumeration.
48+
// If the property name is an own property of the base object, the read won't
49+
// return Object.prototype.
50+
not read = any(EnumeratedPropName n).getASourceProp() and
51+
// Exclude cases where the read has no prototype, or a prototype other than Object.prototype.
52+
not read = prototypeLessObject().getAPropertyRead() and
53+
// Exclude cases where this property has just been assigned to
54+
not read.(DynamicPropRead).hasDominatingAssignment()
55+
)
56+
or
57+
// Same as above, but for property projection.
58+
exists(PropertyProjection proj |
59+
proj.isSingletonProjection() and
60+
pred = proj.getASelector() and
61+
succ = proj and
62+
inlbl.isTaint() and
63+
outlbl instanceof ObjectPrototype
64+
)
65+
}
66+
67+
override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
68+
// Don't propagate the receiver into method calls, as the method lookup will fail on Object.prototype.
69+
node = any(DataFlow::MethodCallNode m).getReceiver() and
70+
lbl instanceof ObjectPrototype
71+
}
72+
73+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
74+
guard instanceof PropertyPresenceCheck or
75+
guard instanceof InExprCheck or
76+
guard instanceof InstanceofCheck or
77+
guard instanceof IsArrayCheck or
78+
guard instanceof EqualityCheck
79+
}
80+
}
81+
82+
/** Gets a data flow node referring to an object created with `Object.create`. */
83+
DataFlow::SourceNode prototypeLessObject() {
84+
result = prototypeLessObject(DataFlow::TypeTracker::end())
85+
}
86+
87+
private DataFlow::SourceNode prototypeLessObject(DataFlow::TypeTracker t) {
88+
t.start() and
89+
// We assume the argument to Object.create is not Object.prototype, since most
90+
// users wouldn't bother to call Object.create in that case.
91+
result = DataFlow::globalVarRef("Object").getAMemberCall("create")
92+
or
93+
// Allow use of AdditionalFlowSteps and AdditionalTaintSteps to track a bit further
94+
exists(DataFlow::Node mid |
95+
prototypeLessObject(t.continue()).flowsTo(mid) and
96+
any(DataFlow::AdditionalFlowStep s).step(mid, result)
97+
)
98+
or
99+
exists(DataFlow::TypeTracker t2 | result = prototypeLessObject(t2).track(t2, t))
100+
}
101+
102+
/** Holds if `Object.prototype` has a member named `prop`. */
103+
private predicate isPropertyPresentOnObjectPrototype(string prop) {
104+
exists(ExternalInstanceMemberDecl decl |
105+
decl.getBaseName() = "Object" and
106+
decl.getName() = prop
107+
)
108+
}
109+
110+
/** A check of form `e.prop` where `prop` is not present on `Object.prototype`. */
111+
private class PropertyPresenceCheck extends TaintTracking::LabeledSanitizerGuardNode,
112+
DataFlow::ValueNode {
113+
override PropAccess astNode;
114+
115+
PropertyPresenceCheck() { not isPropertyPresentOnObjectPrototype(astNode.getPropertyName()) }
116+
117+
override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
118+
e = astNode.getBase() and
119+
outcome = true and
120+
label instanceof ObjectPrototype
121+
}
122+
}
123+
124+
/** A check of form `"prop" in e` where `prop` is not present on `Object.prototype`. */
125+
private class InExprCheck extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::ValueNode {
126+
override InExpr astNode;
127+
128+
InExprCheck() {
129+
not isPropertyPresentOnObjectPrototype(astNode.getLeftOperand().getStringValue())
130+
}
131+
132+
override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
133+
e = astNode.getRightOperand() and
134+
outcome = true and
135+
label instanceof ObjectPrototype
136+
}
137+
}
138+
139+
/** A check of form `e instanceof X`, which is always false for `Object.prototype`. */
140+
private class InstanceofCheck extends TaintTracking::LabeledSanitizerGuardNode,
141+
DataFlow::ValueNode {
142+
override InstanceofExpr astNode;
143+
144+
override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
145+
e = astNode.getLeftOperand() and
146+
outcome = true and
147+
label instanceof ObjectPrototype
148+
}
149+
}
150+
151+
/** A call to `Array.isArray`, which is false for `Object.prototype`. */
152+
private class IsArrayCheck extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::CallNode {
153+
IsArrayCheck() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") }
154+
155+
override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
156+
e = getArgument(0).asExpr() and
157+
outcome = true and
158+
label instanceof ObjectPrototype
159+
}
160+
}
161+
162+
/**
163+
* Sanitizer guard of form `x !== "__proto__"`.
164+
*/
165+
private class EqualityCheck extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
166+
override EqualityTest astNode;
167+
168+
EqualityCheck() { astNode.getAnOperand().getStringValue() = "__proto__" }
169+
170+
override predicate sanitizes(boolean outcome, Expr e) {
171+
e = astNode.getAnOperand() and
172+
outcome = astNode.getPolarity().booleanNot()
173+
}
174+
}
175+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* Provides sources, sinks, and sanitizers for reasoning about assignments
3+
* that my cause prototype pollution.
4+
*/
5+
6+
private import javascript
7+
8+
/**
9+
* Provides sources, sinks, and sanitizers for reasoning about assignments
10+
* that my cause prototype pollution.
11+
*/
12+
module PrototypePollutingAssignment {
13+
/**
14+
* A data flow source for untrusted data from which the special `__proto__` property name may be arise.
15+
*/
16+
abstract class Source extends DataFlow::Node { }
17+
18+
/**
19+
* A data flow sink for prototype-polluting assignments or untrusted property names.
20+
*/
21+
abstract class Sink extends DataFlow::Node {
22+
/**
23+
* The flow label relevant for this sink.
24+
*
25+
* Use the `taint` label for untrusted property names, and the `ObjectPrototype` label for
26+
* object mutations.
27+
*/
28+
abstract DataFlow::FlowLabel getAFlowLabel();
29+
}
30+
31+
/**
32+
* A sanitizer for untrusted property names.
33+
*/
34+
abstract class Sanitizer extends DataFlow::Node { }
35+
36+
/** Flow label representing the `Object.prototype` value. */
37+
abstract class ObjectPrototype extends DataFlow::FlowLabel {
38+
ObjectPrototype() { this = "Object.prototype" }
39+
}
40+
41+
/** The base of an assignment or extend call, as a sink for `Object.prototype` references. */
42+
private class DefaultSink extends Sink {
43+
DefaultSink() {
44+
this = any(DataFlow::PropWrite write).getBase()
45+
or
46+
this = any(ExtendCall c).getDestinationOperand()
47+
}
48+
49+
override DataFlow::FlowLabel getAFlowLabel() { result instanceof ObjectPrototype }
50+
}
51+
52+
/** A remote flow source or location.{hash,search} as a taint source. */
53+
private class DefaultSource extends Source {
54+
DefaultSource() {
55+
this instanceof RemoteFlowSource
56+
or
57+
this = DOM::locationRef().getAPropertyRead(["hash", "search"])
58+
}
59+
}
60+
}

0 commit comments

Comments
 (0)