🔒 Fixes prototype exposure (#22)

* 🔒 Fixes prototype pollution issue * 🔒 Tests for protoype and constructor * 🐛 Updates pnpm action
ekwoka · Jul 29, 2024 · 5163b1b · 5163b1b
1 parent c98cb93
commit 5163b1b
Show file tree

Hide file tree

Showing 5 changed files with 2,952 additions and 2,484 deletions.
diff --git a/.github/workflows/test-pr.yml b/.github/workflows/test-pr.yml
@@ -28,7 +28,7 @@ jobs:
         with:
           node-version: 20
 
-      - uses: pnpm/action-setup@v2.4.0
+      - uses: pnpm/action-setup@v4
         name: Install pnpm
         id: pnpm-install
         with:
@@ -72,7 +72,7 @@ jobs:
         with:
           node-version: 20
 
-      - uses: pnpm/action-setup@v2.4.0
+      - uses: pnpm/action-setup@v4
         name: Install pnpm
         id: pnpm-install
         with:
@@ -119,7 +119,7 @@ jobs:
         with:
           node-version: 20
 
-      - uses: pnpm/action-setup@v2.4.0
+      - uses: pnpm/action-setup@v4
         name: Install pnpm
         id: pnpm-install
         with:
@@ -163,7 +163,7 @@ jobs:
         with:
           node-version: 20
 
-      - uses: pnpm/action-setup@v2.4.0
+      - uses: pnpm/action-setup@v4
         name: Install pnpm
         id: pnpm-install
         with:

diff --git a/packages/params/src/pathresolve.ts b/packages/params/src/pathresolve.ts
@@ -5,10 +5,11 @@ export const objectAtPath = (
 ) => {
   while (keys.length) {
     const key = keys.shift()!;
+    if (isForbidden(key)) return null;
 
     // This is where we fill in empty arrays/objects allong the way to the assigment...
     if (data[key] === undefined)
-      data[key] = isNaN(Number(keys[0] ?? final)) ? {} : [];
+      data[key] = isNaN(Number(keys[0] ?? final)) ? Object.create({}) : [];
     data = data[key] as Record<string, unknown>;
     // Keep deferring assignment until the full key is built up...
   }
@@ -22,8 +23,10 @@ export const insertDotNotatedValueIntoData = (
 ) => {
   const keys = path.split('.');
   const final = keys.pop()!;
-  data = objectAtPath(keys, data, final);
-  data[final] = value;
+  const interimdata = objectAtPath(keys, data, final);
+  return !interimdata || isForbidden(final)
+    ? undefined
+    : (interimdata[final] = value);
 };
 
 export const retrieveDotNotatedValueFromData = (
@@ -32,8 +35,8 @@ export const retrieveDotNotatedValueFromData = (
 ) => {
   const keys = path.split('.');
   const final = keys.pop()!;
-  data = objectAtPath(keys, data, final);
-  return data[final];
+  const interimdata = objectAtPath(keys, data, final);
+  return !interimdata || isForbidden(final) ? undefined : interimdata[final];
 };
 
 export const deleteDotNotatedValueFromData = (
@@ -42,8 +45,8 @@ export const deleteDotNotatedValueFromData = (
 ) => {
   const keys = path.split('.');
   const final = keys.pop()!;
-  data = objectAtPath(keys, data, final);
-  delete data[final];
+  const interimdata = objectAtPath(keys, data, final);
+  if (interimdata && !isForbidden(final)) delete data[final];
 };
 
 if (import.meta.vitest) {
@@ -74,3 +77,6 @@ if (import.meta.vitest) {
     });
   });
 }
+
+export const isForbidden = (key: string) => forbiddenKeys.includes(key);
+const forbiddenKeys = ['__proto__', 'constructor', 'prototype'];
diff --git a/packages/params/src/querystring.ts b/packages/params/src/querystring.ts
@@ -1,4 +1,4 @@
-import { insertDotNotatedValueIntoData } from './pathresolve';
+import { insertDotNotatedValueIntoData, isForbidden } from './pathresolve';
 
 /**
  * Converts Objects to bracketed query strings
@@ -36,12 +36,13 @@ export const buildQueryStringEntries = (
 };
 
 export const fromQueryString = (queryString: string) => {
-  const data: Record<string, unknown> = {};
+  const data: Record<string, unknown> = Object.create(null);
   if (queryString === '') return data;
 
   const entries = new URLSearchParams(queryString).entries();
 
   for (const [key, value] of entries) {
+    if (isForbidden(key)) continue;
     // Query string params don't always have values... (`?foo=`)
     if (!value) continue;
 
@@ -81,5 +82,30 @@ if (import.meta.vitest) {
         expect(fromQueryString(str)).toEqual(obj);
       },
     );
+    it.each(['__proto__', 'constructor', 'prototype'])(
+      'doesnt parse %s',
+      (key) => {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const thing: any = fromQueryString(`hello=world&${key}[fizz]=bar`);
+        expect(thing).toEqual({ hello: `world` });
+        expect(thing[key]?.fizz).toBeUndefined();
+        expect(fromQueryString(`foo[${key}]=bar&hello=world`)).toEqual({
+          foo: {},
+          hello: `world`,
+        });
+        expect(
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          (fromQueryString(`foo[${key}]=bar&hello=world`) as any).foo?.[key],
+        ).not.toEqual(`bar`);
+        expect(
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          (fromQueryString(`foo[0]=test&foo[${key}]=bar&hello=world`) as any)
+            .foo?.[key],
+        ).not.toEqual(`bar`);
+        expect(
+          fromQueryString(`foo[0]=test&foo[${key}]=bar&hello=world`),
+        ).toEqual({ foo: [`test`], hello: `world` });
+      },
+    );
   });
 }