From 790eb17a60c90a6208afdf773f150ba47153edf9 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Mon, 14 Sep 2026 17:47:07 +0700 Subject: [PATCH 1/3] fix(openapi): prevent prototype pollution through object values in bracket notation The deserializer creates its own containers as NullProtoObj or arrays, but a value coming from the input that passes isPlainObject was kept and traversed. A `__proto__` segment on such a value resolved to Object.prototype through the prototype chain, and isPlainObject accepts it (its own proto is null), so the final write landed on the global prototype. Read and write own properties only: getOwn for reads, setOwn for writes, and Object.hasOwn instead of `in`. Export setOwn from @orpc/shared, which already used it internally for set() and clone(). --- packages/openapi/src/bracket-notation.test.ts | 22 +++++++++ packages/openapi/src/bracket-notation.ts | 47 ++++++++++--------- packages/shared/src/object.ts | 2 +- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/packages/openapi/src/bracket-notation.test.ts b/packages/openapi/src/bracket-notation.test.ts index e6e9af973..c0e5e20c4 100644 --- a/packages/openapi/src/bracket-notation.test.ts +++ b/packages/openapi/src/bracket-notation.test.ts @@ -410,6 +410,28 @@ describe('bracket notation serializer', () => { expect(result.constructor.prototype.polluted).toBe('x') expect(result.a.constructor.prototype.polluted).toBe('y') }) + + it('can prevent prototype pollution attack through object values', () => { + /* eslint-disable no-proto, no-restricted-properties */ + const result = serializer.deserialize([ + ['a', {}], + ['a[__proto__][polluted]', '1'], + ['b', { c: '2' }], + ['b[constructor][prototype][polluted]', '3'], + ]) as any + + // global prototypes must be completely unaffected + expect(({} as any).polluted).toBeUndefined() + expect((Object.prototype as any).polluted).toBeUndefined() + + // dangerous keys are stored as plain data on the object value, not as real prototype links + expect(result.a.__proto__).toEqual({ polluted: '1' }) + expect(result.a.polluted).toBeUndefined() + expect(result.b.c).toBe('2') + expect(result.b.constructor.prototype.polluted).toBe('3') + expect(result.b.polluted).toBeUndefined() + /* eslint-enable no-proto, no-restricted-properties */ + }) }) it.each([ diff --git a/packages/openapi/src/bracket-notation.ts b/packages/openapi/src/bracket-notation.ts index 6fc8b9503..a8e655b30 100644 --- a/packages/openapi/src/bracket-notation.ts +++ b/packages/openapi/src/bracket-notation.ts @@ -1,5 +1,5 @@ import type { Segment } from '@orpc/shared' -import { isPlainObject, NullProtoObj } from '@orpc/shared' +import { getOwn, isPlainObject, NullProtoObj, setOwn } from '@orpc/shared' export type BracketNotationSerializeResult = [string, unknown][] @@ -68,42 +68,45 @@ export class BracketNotationSerializer { for (let i = 0; i < segments.length; i++) { const segment = segments[i]! - if (!Array.isArray(currentRef[nextSegment]) && !isPlainObject(currentRef[nextSegment])) { - currentRef[nextSegment] = [] + // Read/write own properties only, so a `__proto__` segment cannot walk into a prototype + let child: any = getOwn(currentRef, nextSegment) + + if (!Array.isArray(child) && !isPlainObject(child)) { + child = [] } if (i !== segments.length - 1) { - if (Array.isArray(currentRef[nextSegment]) && !internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex)) { - if (arrayPushStyles.has(currentRef[nextSegment])) { - arrayPushStyles.delete(currentRef[nextSegment]) - currentRef[nextSegment] = internalPushStyleArrayToObject(currentRef[nextSegment]) + if (Array.isArray(child) && !internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex)) { + if (arrayPushStyles.delete(child)) { + child = internalPushStyleArrayToObject(child) } else { - currentRef[nextSegment] = internalArrayToObject(currentRef[nextSegment]) + child = internalArrayToObject(child) } } } else { - if (Array.isArray(currentRef[nextSegment])) { + if (Array.isArray(child)) { if (segment === '') { - if (currentRef[nextSegment].length && !arrayPushStyles.has(currentRef[nextSegment])) { - currentRef[nextSegment] = internalArrayToObject(currentRef[nextSegment]) + if (child.length && !arrayPushStyles.has(child)) { + child = internalArrayToObject(child) } } else { - if (arrayPushStyles.has(currentRef[nextSegment])) { - arrayPushStyles.delete(currentRef[nextSegment]) - currentRef[nextSegment] = internalPushStyleArrayToObject(currentRef[nextSegment]) + if (arrayPushStyles.delete(child)) { + child = internalPushStyleArrayToObject(child) } else if (!internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex)) { - currentRef[nextSegment] = internalArrayToObject(currentRef[nextSegment]) + child = internalArrayToObject(child) } } } } - currentRef = currentRef[nextSegment] + setOwn(currentRef, nextSegment, child) + + currentRef = child nextSegment = segment } @@ -111,16 +114,18 @@ export class BracketNotationSerializer { arrayPushStyles.add(currentRef) currentRef.push(value) } - else if (nextSegment in currentRef) { - if (Array.isArray(currentRef[nextSegment])) { - currentRef[nextSegment].push(value) + else if (Object.hasOwn(currentRef, nextSegment)) { + const current = getOwn(currentRef, nextSegment) + + if (Array.isArray(current)) { + current.push(value) } else { - currentRef[nextSegment] = [currentRef[nextSegment], value] + setOwn(currentRef, nextSegment, [current, value]) } } else { - currentRef[nextSegment] = value + setOwn(currentRef, nextSegment, value) } } diff --git a/packages/shared/src/object.ts b/packages/shared/src/object.ts index 8f26148fa..a1617ea79 100644 --- a/packages/shared/src/object.ts +++ b/packages/shared/src/object.ts @@ -121,7 +121,7 @@ export function set( /** * Sets `object[key]`, defining `__proto__` as an own property instead of re-parenting the object. */ -function setOwn(object: object, key: PropertyKey, value: unknown): void { +export function setOwn(object: object, key: PropertyKey, value: unknown): void { if (key === '__proto__') { Object.defineProperty(object, key, { value, From a326989bc02e7d0caef81f1291ae60792ed1402c Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Mon, 14 Sep 2026 21:06:27 +0700 Subject: [PATCH 2/3] refactor(openapi): simplify the bracket notation deserializer walk Collapse the four array-conversion branches into a single decision, so the array-or-object rule and its two converters live in one place instead of five. Drop the `ref: { value }` wrapper: the root is always a NullProtoObj, so the first loop iteration could only ever write the root back onto itself. The walk now starts at the first segment and returns the root directly. Write the container back only when it changed, which keeps re-walked paths free of pointless stores and stops a literal `__proto__` segment from re-running Object.defineProperty on every entry. --- packages/openapi/src/bracket-notation.test.ts | 1 - packages/openapi/src/bracket-notation.ts | 63 +++++++------------ 2 files changed, 24 insertions(+), 40 deletions(-) diff --git a/packages/openapi/src/bracket-notation.test.ts b/packages/openapi/src/bracket-notation.test.ts index c0e5e20c4..6281418aa 100644 --- a/packages/openapi/src/bracket-notation.test.ts +++ b/packages/openapi/src/bracket-notation.test.ts @@ -420,7 +420,6 @@ describe('bracket notation serializer', () => { ['b[constructor][prototype][polluted]', '3'], ]) as any - // global prototypes must be completely unaffected expect(({} as any).polluted).toBeUndefined() expect((Object.prototype as any).polluted).toBeUndefined() diff --git a/packages/openapi/src/bracket-notation.ts b/packages/openapi/src/bracket-notation.ts index a8e655b30..de47ecde9 100644 --- a/packages/openapi/src/bracket-notation.ts +++ b/packages/openapi/src/bracket-notation.ts @@ -52,59 +52,44 @@ export class BracketNotationSerializer { } deserialize(serialized: BracketNotationSerializeResult): Record { - if (serialized.length === 0) { - return new NullProtoObj() // Prevent Prototype Pollution with NullProtoObj - } - + // A caller-supplied object value can become a container for deeper paths, and unlike + // `NullProtoObj` it carries a real prototype, so accesses below stay own-property only. const arrayPushStyles = new WeakSet() - const ref: { value: Record } = { value: new NullProtoObj() } // Prevent Prototype Pollution with NullProtoObj + const root: Record = new NullProtoObj() for (const [path, value] of serialized) { const segments = this.parsePath(path) - let currentRef: any = ref - let nextSegment: string = 'value' + let currentRef: any = root + let nextSegment: string = segments[0]! - for (let i = 0; i < segments.length; i++) { + for (let i = 1; i < segments.length; i++) { const segment = segments[i]! + const isLast = i === segments.length - 1 - // Read/write own properties only, so a `__proto__` segment cannot walk into a prototype - let child: any = getOwn(currentRef, nextSegment) + const existing: any = getOwn(currentRef, nextSegment) + let child: any = existing if (!Array.isArray(child) && !isPlainObject(child)) { child = [] } - if (i !== segments.length - 1) { - if (Array.isArray(child) && !internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex)) { - if (arrayPushStyles.delete(child)) { - child = internalPushStyleArrayToObject(child) - } - else { - child = internalArrayToObject(child) - } - } - } - else { - if (Array.isArray(child)) { - if (segment === '') { - if (child.length && !arrayPushStyles.has(child)) { - child = internalArrayToObject(child) - } - } - else { - if (arrayPushStyles.delete(child)) { - child = internalPushStyleArrayToObject(child) - } - - else if (!internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex)) { - child = internalArrayToObject(child) - } - } + if (Array.isArray(child)) { + const isPushStyle = arrayPushStyles.has(child) + + const canStayArray = segment === '' + ? isLast && (isPushStyle || child.length === 0) + : internalIsValidArrayIndex(segment, this.maxExplicitDeserializingArrayIndex) && !(isLast && isPushStyle) + + if (!canStayArray) { + arrayPushStyles.delete(child) + child = isPushStyle ? internalPushStyleArrayToObject(child) : internalArrayToObject(child) } } - setOwn(currentRef, nextSegment, child) + if (child !== existing) { + setOwn(currentRef, nextSegment, child) + } currentRef = child nextSegment = segment @@ -115,7 +100,7 @@ export class BracketNotationSerializer { currentRef.push(value) } else if (Object.hasOwn(currentRef, nextSegment)) { - const current = getOwn(currentRef, nextSegment) + const current = currentRef[nextSegment] if (Array.isArray(current)) { current.push(value) @@ -129,7 +114,7 @@ export class BracketNotationSerializer { } } - return ref.value + return root } stringifyPath(segments: readonly Segment[]): string { From 79dad15c9992edcd1533d47f95ef09259b895dd5 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Mon, 14 Sep 2026 21:06:31 +0700 Subject: [PATCH 3/3] test(shared): cover setOwn and the findDeepMatches skip branch setOwn is now exported, so test it directly: ordinary writes, symbol and number keys, array indexes, and the __proto__ case, including that the defined property stays writable, enumerable and configurable. Enumerability is what lets a deserialized __proto__ key survive a for...in round trip. Also feed findDeepMatches a value that is neither a match, an array nor a plain object, bringing object.ts to full branch coverage. --- packages/shared/src/object.test.ts | 81 +++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/object.test.ts b/packages/shared/src/object.test.ts index 83639b88e..4b47c8c88 100644 --- a/packages/shared/src/object.test.ts +++ b/packages/shared/src/object.test.ts @@ -1,7 +1,7 @@ import * as a from 'arktype' import * as v from 'valibot' import z from 'zod' -import { bindMethods, clone, findDeepMatches, get, getConstructor, getConstructors, getOwn, isPlainObject, isPropertyKey, mergeTwoLevels, NullProtoObj, omit, set } from './object' +import { bindMethods, clone, findDeepMatches, get, getConstructor, getConstructors, getOwn, isPlainObject, isPropertyKey, mergeTwoLevels, NullProtoObj, omit, set, setOwn } from './object' it('findDeepMatches', () => { const { maps, values } = findDeepMatches(v => typeof v === 'string', { @@ -16,6 +16,7 @@ it('findDeepMatches', () => { 'v4', ], }, + ignored: 42, }) expect(maps).toEqual([ @@ -239,6 +240,84 @@ describe('set', () => { }) }) +describe('setOwn', () => { + it('sets a value', () => { + const root: Record = {} + setOwn(root, 'a', 1) + expect(root).toEqual({ a: 1 }) + }) + + it('overwrites an existing value', () => { + const root: Record = { a: 1 } + setOwn(root, 'a', 2) + expect(root).toEqual({ a: 2 }) + }) + + it('supports symbol and number keys', () => { + const root: Record = {} + const sym = Symbol('key') + + setOwn(root, sym, 'sym-value') + setOwn(root, 0, 'number-value') + + expect(root[sym]).toBe('sym-value') + expect(root[0]).toBe('number-value') + }) + + it('writes through to an array index', () => { + const root: unknown[] = [] + setOwn(root, 0, 'value') + + expect(root).toEqual(['value']) + expect(root.length).toBe(1) + }) + + it('sets __proto__ as an own property instead of changing the prototype', () => { + const root: Record = {} + setOwn(root, '__proto__', 'value') + + expect(Object.getPrototypeOf(root)).toBe(Object.prototype) + expect(getOwn(root, '__proto__')).toBe('value') + // eslint-disable-next-line no-proto, no-restricted-properties + expect((root as any).__proto__).toBe('value') + }) + + it('does not pollute the prototype via __proto__', () => { + const root: Record = {} + setOwn(root, '__proto__', { polluted: 'yes' }) + + expect(({} as Record).polluted).toBeUndefined() + expect((Object.prototype as any).polluted).toBeUndefined() + expect(Object.getPrototypeOf(root)).toBe(Object.prototype) + expect(root.polluted).toBeUndefined() + }) + + it('sets __proto__ on a null prototype object', () => { + const root = new NullProtoObj>() + setOwn(root, '__proto__', 'value') + + expect(Object.getPrototypeOf(root)).toBe(Object.getPrototypeOf(new NullProtoObj())) + expect(getOwn(root, '__proto__')).toBe('value') + }) + + it('defines __proto__ as a writable, enumerable and configurable property', () => { + const root: Record = {} + setOwn(root, '__proto__', 'value') + + expect(Object.getOwnPropertyDescriptor(root, '__proto__')).toEqual({ + value: 'value', + writable: true, + enumerable: true, + configurable: true, + }) + + expect(Object.keys(root)).toEqual(['__proto__']) + + setOwn(root, '__proto__', 'updated') + expect(getOwn(root, '__proto__')).toBe('updated') + }) +}) + describe('mergeTwoLevels', () => { it('merges the root level', () => { expect(mergeTwoLevels({ a: 1, shared: 'first' }, { b: 2, shared: 'second' })).toEqual({