fix new proto key injection

jdeniau · jdeniau · commit 94bcd3c79972 · 2026-03-03T22:10:57.000Z
diff --git a/__tests__/functional/set.ts b/__tests__/functional/set.ts
@@ -0,0 +1,69 @@
+import { describe, expect, it } from '@jest/globals';
+import { set } from 'immutable';
+
+describe('set', () => {
+  it('for immutable structure', () => {
+    const originalArray = ['dog', 'frog', 'cat'];
+    expect(set(originalArray, 1, 'cow')).toEqual(['dog', 'cow', 'cat']);
+    expect(set(originalArray, 4, 'cow')).toEqual([
+      'dog',
+      'frog',
+      'cat',
+      undefined,
+      'cow',
+    ]);
+    expect(originalArray).toEqual(['dog', 'frog', 'cat']);
+
+    const originalObject = { x: 123, y: 456 };
+    expect(set(originalObject, 'x', 789)).toEqual({ x: 789, y: 456 });
+    expect(set(originalObject, 'z', 789)).toEqual({ x: 123, y: 456, z: 789 });
+    expect(originalObject).toEqual({ x: 123, y: 456 });
+  });
+
+  it('for Array', () => {
+    const originalArray = ['dog', 'frog', 'cat'];
+    expect(set(originalArray, 1, 'cow')).toEqual(['dog', 'cow', 'cat']);
+    expect(set(originalArray, 4, 'cow')).toEqual([
+      'dog',
+      'frog',
+      'cat',
+      undefined,
+      'cow',
+    ]);
+    expect(originalArray).toEqual(['dog', 'frog', 'cat']);
+  });
+
+  it('for plain objects', () => {
+    const originalObject = { x: 123, y: 456 };
+    expect(set(originalObject, 'x', 789)).toEqual({ x: 789, y: 456 });
+    expect(set(originalObject, 'z', 789)).toEqual({ x: 123, y: 456, z: 789 });
+    expect(originalObject).toEqual({ x: 123, y: 456 });
+  });
+
+  it('is not sensible to prototype pollution via set on plain object', () => {
+    type User = { user: string; admin?: boolean };
+
+    const obj: User = { user: 'Alice' };
+    // Setting __proto__ key should not change the returned object's prototype chain
+    // @ts-expect-error -- intentionally setting __proto__ to test prototype pollution
+    const result = set(obj, '__proto__', { admin: true });
+
+    // The returned copy should NOT have 'admin' accessible via prototype
+    // @ts-expect-error -- testing prototype pollution
+    expect(result.admin).toBeUndefined();
+  });
+
+  it('is not sensible to prototype pollution via set with JSON.parse source', () => {
+    type User = { user: string; admin?: boolean };
+
+    // JSON.parse creates __proto__ as an own property
+    const malicious = JSON.parse(
+      '{"user":"Eve","__proto__":{"admin":true}}'
+    ) as User;
+    // set on an object that already carries __proto__ from JSON.parse
+    const result = set(malicious, 'user', 'Alice');
+
+    // The returned copy should NOT have 'admin' accessible via prototype pollution
+    expect(result.admin).toBeUndefined();
+  });
+});
diff --git a/__tests__/functional/update.ts b/__tests__/functional/update.ts
@@ -0,0 +1,64 @@
+import { describe, expect, it } from '@jest/globals';
+import { update } from 'immutable';
+
+describe('update', () => {
+  it('for immutable structure', () => {
+    const originalArray = ['dog', 'frog', 'cat'];
+    expect(update(originalArray, 1, val => val?.toUpperCase())).toEqual([
+      'dog',
+      'FROG',
+      'cat',
+    ]);
+    expect(originalArray).toEqual(['dog', 'frog', 'cat']);
+
+    const originalObject = { x: 123, y: 456 };
+    expect(update(originalObject, 'x', val => val * 6)).toEqual({
+      x: 738,
+      y: 456,
+    });
+    expect(originalObject).toEqual({ x: 123, y: 456 });
+  });
+
+  it('for Array', () => {
+    const originalArray = ['dog', 'frog', 'cat'];
+    expect(update(originalArray, 1, val => val?.toUpperCase())).toEqual([
+      'dog',
+      'FROG',
+      'cat',
+    ]);
+    expect(originalArray).toEqual(['dog', 'frog', 'cat']);
+  });
+
+  it('for plain objects', () => {
+    const originalObject = { x: 123, y: 456 };
+    expect(update(originalObject, 'x', val => val * 6)).toEqual({
+      x: 738,
+      y: 456,
+    });
+    expect(originalObject).toEqual({ x: 123, y: 456 });
+  });
+
+  it('is not sensible to prototype pollution via update on plain object', () => {
+    type User = { user: string; admin?: boolean };
+
+    const obj: User = { user: 'Alice' };
+    // @ts-expect-error -- intentionally setting __proto__ to test prototype pollution
+    const result = update(obj, '__proto__', () => ({
+      admin: true,
+    })) as unknown as User;
+
+    // The returned copy should NOT have 'admin' accessible via prototype
+    expect(result.admin).toBeUndefined();
+  });
+
+  it('is not sensible to prototype pollution via update with JSON.parse source', () => {
+    type User = { user: string; admin?: boolean };
+
+    // JSON.parse creates __proto__ as an own property
+    const malicious = JSON.parse('{"user":"Eve","__proto__":{"admin":true}}');
+    const result = update(malicious, 'user', () => 'Alice') as User;
+
+    // The returned copy (via shallowCopy) should NOT have 'admin' via prototype
+    expect(result.admin).toBeUndefined();
+  });
+});
diff --git a/__tests__/merge.ts b/__tests__/merge.ts
@@ -357,6 +357,7 @@ describe('merge', () => {
 
     expect(r6.profile.admin).toBeUndefined();
 
+    // @ts-expect-error -- testing prototype pollution
     expect({}.admin).toBeUndefined(); // Confirm NOT global too
   });
 });
diff --git a/__tests__/updateIn.ts b/__tests__/updateIn.ts
@@ -368,4 +368,36 @@ describe('updateIn', () => {
       );
     });
   });
+
+  describe('prototype pollution', () => {
+    it('setIn on plain object with __proto__ key should not pollute returned object', () => {
+      type User = { profile: { bio: string }; admin?: boolean };
+
+      const obj: User = { profile: { bio: 'Hello' } };
+      const result = setIn(obj, ['__proto__', 'admin'], true);
+
+      // The returned object should NOT have 'admin' accessible via prototype
+      expect(result.admin).toBeUndefined();
+    });
+
+    it('setIn on plain object with nested __proto__ key should not pollute returned object', () => {
+      type User = { profile: { bio: string; admin?: boolean } };
+
+      const obj: User = { profile: { bio: 'Hello' } };
+      const result = setIn(obj, ['profile', '__proto__', 'admin'], true);
+
+      // The nested object should NOT have 'admin' accessible via prototype
+      expect(result.profile.admin).toBeUndefined();
+    });
+
+    it('updateIn on plain object with __proto__ key should not pollute returned object', () => {
+      type User = { profile: { bio: string }; admin?: boolean };
+
+      const obj: User = { profile: { bio: 'Hello' } };
+      const result = updateIn(obj, ['__proto__', 'admin'], () => true);
+
+      // The returned object should NOT have 'admin' accessible via prototype
+      expect(result.admin).toBeUndefined();
+    });
+  });
 });
diff --git a/__tests__/utils/shallowCopy.ts b/__tests__/utils/shallowCopy.ts
@@ -0,0 +1,56 @@
+import { describe, it, expect } from '@jest/globals';
+import shallowCopy from '../../src/utils/shallowCopy';
+
+describe('shallowCopy', () => {
+  it('copies a plain object', () => {
+    const obj = { a: 1, b: 2 };
+    const copy = shallowCopy(obj);
+    expect(copy).toEqual({ a: 1, b: 2 });
+    expect(copy).not.toBe(obj);
+  });
+
+  it('copies an array', () => {
+    const arr = [1, 2, 3];
+    const copy = shallowCopy(arr);
+    expect(copy).toEqual([1, 2, 3]);
+    expect(copy).not.toBe(arr);
+  });
+
+  it('should not propagate __proto__ key from source object', () => {
+    type User = { user: string; admin?: boolean };
+
+    // @ts-expect-error -- testing prototype pollution
+    delete Object.prototype.admin;
+
+    // JSON.parse creates an own property named "__proto__" (not the actual prototype)
+    const malicious = JSON.parse('{"user":"Eve","__proto__":{"admin":true}}');
+
+    const copy = shallowCopy(malicious);
+
+    // The copy should NOT have admin on its prototype chain
+    expect((copy as User).admin).toBeUndefined();
+
+    // Global Object prototype should NOT be polluted
+    expect(({} as User).admin).toBeUndefined();
+
+    // @ts-expect-error -- cleanup
+    delete Object.prototype.admin;
+  });
+
+  it('should not propagate constructor key from source object', () => {
+    type User = { user: string; admin?: boolean };
+
+    const malicious: User = {
+      user: 'Eve',
+      // @ts-expect-error -- intentionally setting constructor to test pollution
+      constructor: { prototype: { admin: true } },
+    };
+
+    const copy = shallowCopy(malicious);
+
+    expect((copy as User).admin).toBeUndefined();
+
+    // The constructor of a plain new object should still be Object
+    expect({}.constructor).toBe(Object);
+  });
+});
diff --git a/src/functional/set.js b/src/functional/set.js
@@ -1,9 +1,14 @@
 import { isImmutable } from '../predicates/isImmutable';
 import hasOwnProperty from '../utils/hasOwnProperty';
 import isDataStructure from '../utils/isDataStructure';
+import { isProtoKey } from '../utils/protoInjection';
 import shallowCopy from '../utils/shallowCopy';
 
 export function set(collection, key, value) {
+  if (isProtoKey(key)) {
+    return collection;
+  }
+
   if (!isDataStructure(collection)) {
     throw new TypeError(
       'Cannot update non-data-structure value: ' + collection
diff --git a/src/utils/protoInjection.js b/src/utils/protoInjection.js
@@ -0,0 +1,5 @@
+export function isProtoKey(key) {
+  return (
+    typeof key === 'string' && (key === '__proto__' || key === 'constructor')
+  );
+}
diff --git a/src/utils/protoInjection.ts b/src/utils/protoInjection.ts
diff --git a/src/utils/shallowCopy.js b/src/utils/shallowCopy.js
@@ -1,12 +1,17 @@
 import arrCopy from './arrCopy';
 import hasOwnProperty from './hasOwnProperty';
+import { isProtoKey } from './protoInjection';
 
 export default function shallowCopy(from) {
   if (Array.isArray(from)) {
     return arrCopy(from);
   }
   const to = {};
   for (const key in from) {
+    if (isProtoKey(key)) {
+      continue;
+    }
+
     if (hasOwnProperty.call(from, key)) {
       to[key] = from[key];
     }
