Skip to content

Commit

Permalink
Prevent prototype pollution in cookie memstore (#283)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
colincasey authored Jun 5, 2023
1 parent f06b72d commit 12d4747
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/memstore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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()}`;
Expand Down
25 changes: 25 additions & 0 deletions test/cookie_jar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 12d4747

Please sign in to comment.