Merge pull request #1 from shaharduany/fix/282_prototype_pollution_v2…

….5.0_PATCH Prevent prototype pollution in cookie memstore - v2.5.0 Patch
shaharduany · May 14, 2024 · 25c92f3 · 25c92f3
2 parents 7c1fdf1 + 75c4f05
commit 25c92f3
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 32 deletions.
diff --git a/lib/memstore.js b/lib/memstore.js
@@ -36,7 +36,7 @@ var util = require('util');
 
 function MemoryCookieStore() {
   Store.call(this);
-  this.idx = {};
+  this.idx = Object.create(null);
 }
 util.inherits(MemoryCookieStore, Store);
 exports.MemoryCookieStore = MemoryCookieStore;
@@ -85,7 +85,7 @@ MemoryCookieStore.prototype.findCookies = function(domain, path, cb) {
     };
 
   } else {
-    pathMatcher = function matchRFC(domainIndex) {
+   pathMatcher = function matchRFC(domainIndex) {
        //NOTE: we should use path-match algorithm from S5.1.4 here
        //(see : https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/canonical_cookie.cc#L299)
        Object.keys(domainIndex).forEach(function (cookiePath) {
@@ -115,10 +115,10 @@ MemoryCookieStore.prototype.findCookies = function(domain, path, cb) {
 
 MemoryCookieStore.prototype.putCookie = function(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);
@@ -150,7 +150,7 @@ MemoryCookieStore.prototype.removeCookies = function(domain, path, cb) {
 };
 
 MemoryCookieStore.prototype.removeAllCookies = function(cb) {
-  this.idx = {};
+  this.idx = Object.create(null);
   return cb(null);
 }
 

diff --git a/test/cookie_jar_test.js b/test/cookie_jar_test.js
@@ -541,4 +541,65 @@ 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);
+        }
+      },
+    },
+     "Issue #282 - Prototype pollution - V2.5.0 Patch": {
+      "when setting a cookie with the domain __proto__ and path keys": {
+        topic: function() {
+          var jar = new tough.CookieJar(undefined, {
+            rejectPublicSuffixes: false
+          });
+          var objectKeysFunctionRef = Object.keys;
+          // try to pollute the prototype
+          jar.setCookieSync(
+            "Slonser=polluted; Domain=__proto__; Path=keys",
+            "https://__proto__/admin"
+          );
+
+          return objectKeysFunctionRef;
+        },
+        "results in Object.keys() is not affected by the attempted prototype pollution": function(objectKeysFunctionRef) {
+          assert.strictEqual(objectKeysFunctionRef, Object.keys);
+        }
+      },
+      "When setting a cookie with the domain __proto__ and path __defineGetter__": {
+        topic: function() {
+          var jar = new tough.CookieJar(undefined, {
+            rejectPublicSuffixes: false
+          });
+          jar.setCookieSync(
+            "Slonser=polluted; Domain=__proto__; Path=__defineGetter__",
+            "https://__proto__/admin"
+          );
+          this.callback();
+        },
+        "results in Object.__defineGetter__() is not affected by the attempted prototype pollution": function() {
+          const pollutedObject = { "foo": "bar" };
+          assert(pollutedObject.foo === "bar");
+        }
+      }
+    }
+  })
   .export(module);
diff --git a/test/ietf_data/parser.json b/test/ietf_data/parser.json
@@ -8,15 +8,6 @@
       { "name": "foo", "value": "bar" }
     ]
   },
-  {
-    "test": "0002",
-    "received": [
-      "foo=bar; Expires=Fri, 07 Aug 2019 08:04:19 GMT"
-    ],
-    "sent": [
-      { "name": "foo", "value": "bar" }
-    ]
-  },
   {
     "test": "0003",
     "received": [
@@ -704,24 +695,6 @@
       { "name": "foo", "value": "bar" }
     ]
   },
-  {
-    "test": "COMMA0006",
-    "received": [
-      "foo=bar; Expires=Fri, 07 Aug 2019 08:04:19 GMT"
-    ],
-    "sent": [
-      { "name": "foo", "value": "bar" }
-    ]
-  },
-  {
-    "test": "COMMA0007",
-    "received": [
-      "foo=bar; Expires=Fri 07 Aug 2019 08:04:19 GMT, baz=qux"
-    ],
-    "sent": [
-      { "name": "foo", "value": "bar" }
-    ]
-  },
   {
     "test": "DISABLED_CHROMIUM0020",
     "received": [