From 97fa1788c2184ff4b87fd3f558b7003f9eeee2b1 Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Sat, 27 Jul 2019 17:39:55 -0400 Subject: [PATCH] Prevent prototype pollution while parsing query strings (#2494) * Prevent prototype pollution while parsing query strings * Update changelog [skip ci] --- docs/change-log.md | 1 + querystring/parse.js | 14 ++++++++++---- querystring/tests/test-parseQueryString.js | 12 ++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/change-log.md b/docs/change-log.md index a6c54fc8..7aae9cf5 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -17,6 +17,7 @@ ### Upcoming... - 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)) +- Fix prototype pollution vulnerability in `m.parseQueryString` ([#2494](https://github.com/MithrilJS/mithril.js/pull/2494) [@isiahmeadows](https://github.com/isiahmeadows)) --> diff --git a/querystring/parse.js b/querystring/parse.js index e8618938..60cf454d 100644 --- a/querystring/parse.js +++ b/querystring/parse.js @@ -1,7 +1,5 @@ "use strict" -// The extra `data` parameter is for if you want to append to an existing -// parameters object. module.exports = function(string) { if (string === "" || string == null) return {} if (string.charAt(0) === "?") string = string.slice(1) @@ -29,9 +27,17 @@ module.exports = function(string) { } level = counters[key]++ } + // Disallow direct prototype pollution + else if (level === "__proto__") break if (isValue) cursor[level] = value - else if (cursor[level] == null) cursor[level] = isNumber ? [] : {} - cursor = cursor[level] + else { + // Read own properties exclusively to disallow indirect + // prototype pollution + value = Object.getOwnPropertyDescriptor(cursor, level) + if (value != null) value = value.value + if (value == null) value = cursor[level] = isNumber ? [] : {} + } + cursor = value } } return data diff --git a/querystring/tests/test-parseQueryString.js b/querystring/tests/test-parseQueryString.js index ab7cb395..844af1b4 100644 --- a/querystring/tests/test-parseQueryString.js +++ b/querystring/tests/test-parseQueryString.js @@ -97,4 +97,16 @@ o.spec("parseQueryString", function() { var data = parseQueryString("a=1&b=2&a=3") o(data).deepEquals({a: "3", b: "2"}) }) + o("doesn't pollute prototype directly, censors `__proto__`", function() { + var prev = Object.prototype.toString + var data = parseQueryString("a=b&__proto__%5BtoString%5D=123") + o(Object.prototype.toString).equals(prev) + o(data).deepEquals({a: "b"}) + }) + o("doesn't pollute prototype indirectly, retains `constructor`", function() { + var prev = Object.prototype.toString + var data = parseQueryString("constructor%5Bprototype%5D%5BtoString%5D=123") + o(Object.prototype.toString).equals(prev) + o(data).deepEquals({a: "b"}) + }) })