Skip to content

Commit 8584a01

Browse files
committed
Add fix from #283
1 parent 2bc6217 commit 8584a01

File tree

3 files changed

+55
-12
lines changed

3 files changed

+55
-12
lines changed

lib/__tests__/cookieJar.spec.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* POSSIBILITY OF SUCH DAMAGE.
3030
*/
3131

32-
import {Cookie, CookieJar, MemoryCookieStore, ParameterError, SerializedCookieJar, Store} from '../cookie'
32+
import { Cookie, CookieJar, MemoryCookieStore, ParameterError, SerializedCookieJar, Store } from "../cookie";
3333

3434
const { objectContaining, assertions } = expect
3535
jest.useFakeTimers()
@@ -1008,6 +1008,24 @@ it('should fix issue #197 - CookieJar().setCookie throws an error when empty coo
10081008
)).rejects.toThrowError('Cookie failed to parse')
10091009
})
10101010

1011+
it('should fix issue #282 - Prototype pollution when setting a cookie with the domain __proto__', async () => {
1012+
const jar = new CookieJar(undefined, {
1013+
rejectPublicSuffixes: false
1014+
});
1015+
// try to pollute the prototype
1016+
jar.setCookieSync(
1017+
"Slonser=polluted; Domain=__proto__; Path=/notauth",
1018+
"https://__proto__/admin"
1019+
);
1020+
jar.setCookieSync(
1021+
"Auth=Lol; Domain=google.com; Path=/notauth",
1022+
"https://google.com/"
1023+
);
1024+
1025+
const pollutedObject = {};
1026+
expect('/notauth' in pollutedObject).toBe(false);
1027+
})
1028+
10111029
// special use domains under a sub-domain
10121030
describe.each([
10131031
"local",

lib/memstore.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@
2929
* POSSIBILITY OF SUCH DAMAGE.
3030
*/
3131
"use strict";
32-
import {Callback, Cookie, createPromiseCallback, pathMatch, permuteDomain} from "./cookie";
33-
import {Store} from './store'
34-
import {getCustomInspectSymbol, getUtilInspect} from './utilHelper'
32+
import { Callback, Cookie, createPromiseCallback, pathMatch, permuteDomain } from "./cookie";
33+
import { Store } from "./store";
34+
import { getCustomInspectSymbol, getUtilInspect } from "./utilHelper";
3535

3636
export class MemoryCookieStore extends Store {
3737
override synchronous: boolean;
@@ -46,7 +46,7 @@ export class MemoryCookieStore extends Store {
4646
constructor() {
4747
super();
4848
this.synchronous = true;
49-
this.idx = {};
49+
this.idx = Object.create(null);
5050
const customInspectSymbol = getCustomInspectSymbol();
5151
if (customInspectSymbol) {
5252
// @ts-ignore
@@ -154,10 +154,10 @@ export class MemoryCookieStore extends Store {
154154
return promiseCallback.promise
155155
}
156156

157-
const domainEntry: { [key: string]: any } = this.idx[domain] ?? {}
157+
const domainEntry: { [key: string]: any } = this.idx[domain] ?? Object.create(null)
158158
this.idx[domain] = domainEntry
159159

160-
const pathEntry: { [key: string]: any } = domainEntry[path] ?? {}
160+
const pathEntry: { [key: string]: any } = domainEntry[path] ?? Object.create(null)
161161
domainEntry[path] = pathEntry
162162

163163
pathEntry[key] = cookie
@@ -225,7 +225,7 @@ export class MemoryCookieStore extends Store {
225225
const promiseCallback = createPromiseCallback<void>(arguments)
226226
const cb = promiseCallback.callback
227227

228-
this.idx = {};
228+
this.idx = Object.create(null);
229229

230230
cb(null);
231231
return promiseCallback.promise
@@ -270,9 +270,9 @@ export class MemoryCookieStore extends Store {
270270
export function inspectFallback(val: { [x: string]: any; }) {
271271
const domains = Object.keys(val);
272272
if (domains.length === 0) {
273-
return "{}";
273+
return "[Object: null prototype] {}";
274274
}
275-
let result = "{\n";
275+
let result = "[Object: null prototype] {\n";
276276
Object.keys(val).forEach((domain, i) => {
277277
result += formatDomain(domain, val[domain]);
278278
if (i < domains.length - 1) {
@@ -286,7 +286,7 @@ export function inspectFallback(val: { [x: string]: any; }) {
286286

287287
function formatDomain(domainName: string, domainValue: { [x: string]: any; }) {
288288
const indent = " ";
289-
let result = `${indent}'${domainName}': {\n`;
289+
let result = `${indent}'${domainName}': [Object: null prototype] {\n`;
290290
Object.keys(domainValue).forEach((path, i, paths) => {
291291
result += formatPath(path, domainValue[path]);
292292
if (i < paths.length - 1) {
@@ -300,7 +300,7 @@ function formatDomain(domainName: string, domainValue: { [x: string]: any; }) {
300300

301301
function formatPath(pathName: string, pathValue: { [x: string]: any; }) {
302302
const indent = " ";
303-
let result = `${indent}'${pathName}': {\n`;
303+
let result = `${indent}'${pathName}': [Object: null prototype] {\n`;
304304
Object.keys(pathValue).forEach((cookieName, i, cookieNames) => {
305305
const cookie = pathValue[cookieName];
306306
result += ` ${cookieName}: ${cookie.inspect()}`;

test/cookie_jar_test.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,4 +810,29 @@ vows
810810
}
811811
}
812812
})
813+
.addBatch({
814+
"Issue #282 - Prototype pollution": {
815+
"when setting a cookie with the domain __proto__": {
816+
topic: function() {
817+
const jar = new tough.CookieJar(undefined, {
818+
rejectPublicSuffixes: false
819+
});
820+
// try to pollute the prototype
821+
jar.setCookieSync(
822+
"Slonser=polluted; Domain=__proto__; Path=/notauth",
823+
"https://__proto__/admin"
824+
);
825+
jar.setCookieSync(
826+
"Auth=Lol; Domain=google.com; Path=/notauth",
827+
"https://google.com/"
828+
);
829+
this.callback();
830+
},
831+
"results in a cookie that is not affected by the attempted prototype pollution": function() {
832+
const pollutedObject = {};
833+
assert(pollutedObject["/notauth"] === undefined);
834+
}
835+
}
836+
}
837+
})
813838
.export(module);

0 commit comments

Comments
 (0)