Skip to content

Commit 3af3c54

Browse files
author
Esben Sparre Andreasen
authored
Merge pull request #1318 from asger-semmle/prototype-pollution-query2
Move prototype pollution query into suite
2 parents 29ae7b5 + ba69e19 commit 3af3c54

File tree

6 files changed

+96
-2
lines changed

6 files changed

+96
-2
lines changed
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
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 through use of an unsafe <em>merge</em> or <em>extend</em> function
16+
to recursively copy properties from an untrusted source object.
17+
Such a call can modify any object reachable from the destination object, and
18+
the built-in <code>Object.prototype</code> is usually reachable through the special properties
19+
<code>__proto__</code> and <code>constructor.prototype</code>.
20+
An attacker can abuse this by sending an object with these property names and thereby modify <code>Object.prototype</code>.
21+
</p>
22+
</overview>
23+
24+
<recommendation>
25+
<p>
26+
Update your library dependencies in order to use a safe version of the <em>merge</em> or <em>extend</em> function.
27+
If your library has no fixed version, switch to another library.
28+
</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>
33+
In the example below, the untrusted value <code>req.query.prefs</code> is parsed as JSON
34+
and then copied into a new object:
35+
</p>
36+
37+
<sample src="examples/PrototypePollution1.js"/>
38+
39+
<p>
40+
Prior to lodash 4.17.11 this would be vulnerable to prototype pollution. An attacker could send the following GET request:
41+
</p>
42+
43+
<pre>GET /news?prefs={"constructor":{"prototype":{"xxx":true}}}</pre>
44+
45+
<p>
46+
This causes the <code>xxx</code> property to be injected on <code>Object.prototype</code>.
47+
Fix this by updating the lodash version:
48+
</p>
49+
50+
<sample src="examples/PrototypePollution_fixed.json"/>
51+
52+
<p>
53+
Note that some web frameworks, such as Express, parse query parameters using extended URL-encoding
54+
by default.
55+
When this is the case, the application may be vulnerable even if not using <code>JSON.parse</code>.
56+
The example below would also be susceptible to prototype pollution:
57+
</p>
58+
59+
<sample src="examples/PrototypePollution2.js"/>
60+
61+
<p>
62+
In the above example, an attacker can cause prototype pollution by sending the following GET request:
63+
</p>
64+
65+
<pre>GET /news?prefs[constructor][prototype][xxx]=true</pre>
66+
</example>
67+
68+
<references>
69+
<li>Prototype pollution attacks:
70+
<a href="https://hackerone.com/reports/380873">lodash</a>,
71+
<a href="https://hackerone.com/reports/454365">jQuery</a>,
72+
<a href="https://hackerone.com/reports/381185">extend</a>,
73+
<a href="https://hackerone.com/reports/430291">just-extend</a>,
74+
<a href="https://hackerone.com/reports/381194">merge.recursive</a>.
75+
</li>
76+
<li>Express:
77+
<a href="https://expressjs.com/en/api.html#express.urlencoded">urlencoded()</a>
78+
</li>
79+
</references>
80+
</qhelp>

javascript/ql/test/query-tests/Security/CWE-400/PrototypePollution.ql renamed to javascript/ql/src/Security/CWE-400/PrototypePollution.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
2-
* @name Prototype Pollution
2+
* @name Prototype pollution
33
* @description Recursively merging a user-controlled object into another object
44
* can allow an attacker to modify the built-in Object prototype.
55
* @kind path-problem
66
* @problem.severity warning
7-
* @precision high
7+
* @precision medium
88
* @id js/prototype-pollution
99
* @tags security
1010
* external/cwe/cwe-250
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
app.get('/news', (req, res) => {
2+
let prefs = lodash.merge({}, JSON.parse(req.query.prefs));
3+
})
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
app.get('/news', (req, res) => {
2+
let config = lodash.merge({}, {
3+
prefs: req.query.prefs
4+
});
5+
})
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"dependencies": {
3+
"lodash": "^4.17.11"
4+
}
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-400/PrototypePollution.ql

0 commit comments

Comments
 (0)