From 12d474791bb856004e858fdb1c47b7608d09cf6e Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Mon, 5 Jun 2023 12:13:22 -0300 Subject: [PATCH] Prevent prototype pollution in cookie memstore (#283) All occurrences of new object creation in `memstore.js` have been changed from `{}` (i.e.; `Object.create(Object.prototype)` to `Object.create(null)` so that we are using object instances that do not have a prototype property that can be polluted. @fixes #282 --- lib/memstore.js | 16 ++++++++-------- test/cookie_jar_test.js | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/lib/memstore.js b/lib/memstore.js index 001ca930..f313bbf9 100644 --- a/lib/memstore.js +++ b/lib/memstore.js @@ -39,7 +39,7 @@ class MemoryCookieStore extends Store { constructor() { super(); this.synchronous = true; - this.idx = {}; + this.idx = Object.create(null); const customInspectSymbol = getCustomInspectSymbol(); if (customInspectSymbol) { this[customInspectSymbol] = this.inspect; @@ -111,10 +111,10 @@ class MemoryCookieStore extends Store { putCookie(cookie, cb) { if (!this.idx[cookie.domain]) { - this.idx[cookie.domain] = {}; + this.idx[cookie.domain] = Object.create(null); } if (!this.idx[cookie.domain][cookie.path]) { - this.idx[cookie.domain][cookie.path] = {}; + this.idx[cookie.domain][cookie.path] = Object.create(null); } this.idx[cookie.domain][cookie.path][cookie.key] = cookie; cb(null); @@ -146,7 +146,7 @@ class MemoryCookieStore extends Store { return cb(null); } removeAllCookies(cb) { - this.idx = {}; + this.idx = Object.create(null); return cb(null); } getAllCookies(cb) { @@ -196,9 +196,9 @@ exports.MemoryCookieStore = MemoryCookieStore; function inspectFallback(val) { const domains = Object.keys(val); if (domains.length === 0) { - return "{}"; + return "[Object: null prototype] {}"; } - let result = "{\n"; + let result = "[Object: null prototype] {\n"; Object.keys(val).forEach((domain, i) => { result += formatDomain(domain, val[domain]); if (i < domains.length - 1) { @@ -212,7 +212,7 @@ function inspectFallback(val) { function formatDomain(domainName, domainValue) { const indent = " "; - let result = `${indent}'${domainName}': {\n`; + let result = `${indent}'${domainName}': [Object: null prototype] {\n`; Object.keys(domainValue).forEach((path, i, paths) => { result += formatPath(path, domainValue[path]); if (i < paths.length - 1) { @@ -226,7 +226,7 @@ function formatDomain(domainName, domainValue) { function formatPath(pathName, pathValue) { const indent = " "; - let result = `${indent}'${pathName}': {\n`; + let result = `${indent}'${pathName}': [Object: null prototype] {\n`; Object.keys(pathValue).forEach((cookieName, i, cookieNames) => { const cookie = pathValue[cookieName]; result += ` ${cookieName}: ${cookie.inspect()}`; diff --git a/test/cookie_jar_test.js b/test/cookie_jar_test.js index 676e50db..50205bce 100644 --- a/test/cookie_jar_test.js +++ b/test/cookie_jar_test.js @@ -809,4 +809,29 @@ vows } } }) + .addBatch({ + "Issue #282 - Prototype pollution": { + "when setting a cookie with the domain __proto__": { + topic: function() { + const jar = new tough.CookieJar(undefined, { + rejectPublicSuffixes: false + }); + // try to pollute the prototype + jar.setCookieSync( + "Slonser=polluted; Domain=__proto__; Path=/notauth", + "https://__proto__/admin" + ); + jar.setCookieSync( + "Auth=Lol; Domain=google.com; Path=/notauth", + "https://google.com/" + ); + this.callback(); + }, + "results in a cookie that is not affected by the attempted prototype pollution": function() { + const pollutedObject = {}; + assert(pollutedObject["/notauth"] === undefined); + } + } + } + }) .export(module);