Skip to content

Commit 97fa178

Browse files
authoredJul 27, 2019
Prevent prototype pollution while parsing query strings (#2494)
* Prevent prototype pollution while parsing query strings * Update changelog [skip ci]
1 parent 48e7fd1 commit 97fa178

File tree

3 files changed

+23
-4
lines changed

3 files changed

+23
-4
lines changed
 

‎docs/change-log.md

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
### Upcoming...
1818
1919
- Ensure vnodes are removed correctly in the face of `onbeforeremove` resolving after new nodes are added ([#2492](https://github.com/MithrilJS/mithril.js/pull/2492) [@isiahmeadows](https://github.com/isiahmeadows))
20+
- Fix prototype pollution vulnerability in `m.parseQueryString` ([#2494](https://github.com/MithrilJS/mithril.js/pull/2494) [@isiahmeadows](https://github.com/isiahmeadows))
2021
2122
-->
2223

‎querystring/parse.js

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
"use strict"
22

3-
// The extra `data` parameter is for if you want to append to an existing
4-
// parameters object.
53
module.exports = function(string) {
64
if (string === "" || string == null) return {}
75
if (string.charAt(0) === "?") string = string.slice(1)
@@ -29,9 +27,17 @@ module.exports = function(string) {
2927
}
3028
level = counters[key]++
3129
}
30+
// Disallow direct prototype pollution
31+
else if (level === "__proto__") break
3232
if (isValue) cursor[level] = value
33-
else if (cursor[level] == null) cursor[level] = isNumber ? [] : {}
34-
cursor = cursor[level]
33+
else {
34+
// Read own properties exclusively to disallow indirect
35+
// prototype pollution
36+
value = Object.getOwnPropertyDescriptor(cursor, level)
37+
if (value != null) value = value.value
38+
if (value == null) value = cursor[level] = isNumber ? [] : {}
39+
}
40+
cursor = value
3541
}
3642
}
3743
return data

‎querystring/tests/test-parseQueryString.js

+12
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,16 @@ o.spec("parseQueryString", function() {
9797
var data = parseQueryString("a=1&b=2&a=3")
9898
o(data).deepEquals({a: "3", b: "2"})
9999
})
100+
o("doesn't pollute prototype directly, censors `__proto__`", function() {
101+
var prev = Object.prototype.toString
102+
var data = parseQueryString("a=b&__proto__%5BtoString%5D=123")
103+
o(Object.prototype.toString).equals(prev)
104+
o(data).deepEquals({a: "b"})
105+
})
106+
o("doesn't pollute prototype indirectly, retains `constructor`", function() {
107+
var prev = Object.prototype.toString
108+
var data = parseQueryString("constructor%5Bprototype%5D%5BtoString%5D=123")
109+
o(Object.prototype.toString).equals(prev)
110+
o(data).deepEquals({a: "b"})
111+
})
100112
})

0 commit comments

Comments
 (0)
Please sign in to comment.