Merge pull request #287 from salesforce/port_283_fix_to_v5

Port 283 fix to v5
salesforce · Jun 19, 2023 · e826e9b · e826e9b
2 parents 1bb83ae + b87dc38
commit e826e9b
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 8 deletions.
diff --git a/lib/__tests__/cookieJar.spec.ts b/lib/__tests__/cookieJar.spec.ts
@@ -1195,6 +1195,24 @@ it('should fix issue #197 - CookieJar().setCookie throws an error when empty coo
   ).rejects.toThrowError('Cookie failed to parse')
 })
 
+it('should fix issue #282 - Prototype pollution when setting a cookie with the domain __proto__', async () => {
+  const jar = new 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/',
+  )
+
+  const pollutedObject = {}
+  expect('/notauth' in pollutedObject).toBe(false)
+})
+
 // special use domains under a sub-domain
 describe.each(['local', 'example', 'invalid', 'localhost', 'test'])(
   'when special use domain is dev.%s',

diff --git a/lib/memstore.ts b/lib/memstore.ts
@@ -52,7 +52,7 @@ export class MemoryCookieStore extends Store {
   constructor() {
     super()
     this.synchronous = true
-    this.idx = {}
+    this.idx = Object.create(null)
     const customInspectSymbol = getCustomInspectSymbol()
     if (customInspectSymbol) {
       // @ts-ignore
@@ -188,10 +188,12 @@ export class MemoryCookieStore extends Store {
       return promiseCallback.promise
     }
 
-    const domainEntry: { [key: string]: any } = this.idx[domain] ?? {}
+    const domainEntry: { [key: string]: any } =
+      this.idx[domain] ?? Object.create(null)
     this.idx[domain] = domainEntry
 
-    const pathEntry: { [key: string]: any } = domainEntry[path] ?? {}
+    const pathEntry: { [key: string]: any } =
+      domainEntry[path] ?? Object.create(null)
     domainEntry[path] = pathEntry
 
     pathEntry[key] = cookie
@@ -290,7 +292,7 @@ export class MemoryCookieStore extends Store {
     const promiseCallback = createPromiseCallback<void>(arguments)
     const cb = promiseCallback.callback
 
-    this.idx = {}
+    this.idx = Object.create(null)
 
     cb(null)
     return promiseCallback.promise
@@ -335,9 +337,9 @@ export class MemoryCookieStore extends Store {
 export function inspectFallback(val: { [x: string]: any }) {
   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) {
@@ -351,7 +353,7 @@ export function inspectFallback(val: { [x: string]: any }) {
 
 function formatDomain(domainName: string, domainValue: { [x: string]: any }) {
   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) {
@@ -365,7 +367,7 @@ function formatDomain(domainName: string, domainValue: { [x: string]: any }) {
 
 function formatPath(pathName: string, pathValue: { [x: string]: any }) {
   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
@@ -810,4 +810,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);