Skip to content

Commit 8a35262

Browse files
authored
fix(json-schema): prevent prototype injection when coercing objects (#1726)
Fixes two prototype chain bugs in the `object` branch of `JsonSchemaCoercer#coerce`. The coercer runs on untrusted input before validation (`SmartCoercionPlugin`), so the keys below are attacker controlled. ### 1. Coerced properties were collected into a plain `{}` `coercedItems['__proto__'] = value` triggers the inherited `__proto__` setter instead of creating an own property: ``` schema: { type: 'object', properties: { a: { type: 'number' } } } input: {"a":"1","__proto__":{"isAdmin":true}} out = { a: 1 } // __proto__ entry dropped from the output out.isAdmin === true // attacker object became the prototype ``` `Object.prototype` is not touched, but the returned value inherits attacker controlled properties, and anything reading them through the prototype chain sees them. Fix: collect into `NullProtoObj`, the helper already used for untrusted keys in `bracket-notation.ts` and `rpc-matcher.ts`. ### 2. `schema.properties?.[key]` returned inherited members For keys like `constructor`, `toString` and `__proto__` the lookup resolves to `Object.prototype` members, which were then used as sub-schemas. They coerce nothing and report `satisfied: true`, so those keys skipped the unknown property path and selected the wrong union branch: ``` anyOf: [{ type: 'object', properties: { a: { type: 'number' } } }, { type: 'object', additionalProperties: { type: 'boolean' } }] { z: 'true' } => { z: true } // correct branch { constructor: 'true' } => { constructor: 'true' } // before: wrong branch, not coerced ``` Fix: guard the lookup with `Object.hasOwn`. ### Verification `prevents prototype injection when coercing objects` covers both cases and fails without the fix. The json-schema, openapi and openapi-client suites pass (412 tests). `tsc -b` and eslint are clean.
1 parent 54ad8ac commit 8a35262

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

packages/json-schema/src/coercer.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,35 @@ describe('jsonSchemaCoercer', () => {
162162
).toEqual({ 0: 123, 1: true })
163163
})
164164

165+
it('prevents prototype injection when coercing objects', () => {
166+
const schema = {
167+
type: 'object',
168+
properties: { a: { type: 'number' } },
169+
additionalProperties: { type: 'boolean' },
170+
} as any
171+
172+
const coerced: any = coercer.coerce(schema, JSON.parse('{"a":"123","__proto__":{"polluted":"true"}}'))
173+
174+
// `__proto__` must stay a normal own property instead of replacing the prototype
175+
expect(coerced.a).toBe(123)
176+
expect(Object.hasOwn(coerced, '__proto__')).toBe(true)
177+
expect(Object.getOwnPropertyDescriptor(coerced, '__proto__')!.value).toEqual({ polluted: 'true' })
178+
expect(coerced.polluted).toBeUndefined()
179+
expect(({} as any).polluted).toBeUndefined()
180+
181+
// `Object.prototype` members must not be used as sub-schemas
182+
const unionSchema = {
183+
anyOf: [
184+
{ type: 'object', properties: { a: { type: 'number' } } },
185+
{ type: 'object', additionalProperties: { type: 'boolean' } },
186+
],
187+
} as any
188+
189+
expect(coercer.coerce(unionSchema, { constructor: 'true' })).toEqual({ constructor: true })
190+
expect(coercer.coerce(unionSchema, { toString: 'true' })).toEqual({ toString: true })
191+
expect(coercer.coerce(unionSchema, JSON.parse('{"__proto__":"true"}'))).toEqual({ ['__proto__']: true })
192+
})
193+
165194
it('can handle union types', () => {
166195
const schema = {
167196
anyOf: [

packages/json-schema/src/coercer.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { JsonSchema } from './types'
2-
import { guard, isObject, toArray } from '@orpc/shared'
2+
import { guard, isObject, NullProtoObj, toArray } from '@orpc/shared'
33
import { JsonSchemaXNativeType } from './types'
44

55
const FLEXIBLE_DATE_FORMAT_REGEX = /^[^-]+-[^-]+-[^-]+$/
@@ -166,14 +166,18 @@ export class JsonSchemaCoercer {
166166

167167
if (isObject(coerced)) {
168168
let shouldUseCoercedItems = false
169-
const coercedItems: Record<string, unknown> = {}
169+
/**
170+
* Use a null-prototype object, keys come from untrusted input
171+
* so `coercedItems[key] = value` must never touch `Object.prototype` members like `__proto__`.
172+
*/
173+
const coercedItems: Record<string, unknown> = new NullProtoObj()
170174

171175
const patternProperties = Object.entries(schema.patternProperties ?? {})
172176
.map(([key, value]) => [new RegExp(key), value] as const)
173177

174178
for (const key in coerced) {
175179
const value = coerced[key]
176-
const subSchema = schema.properties?.[key]
180+
const subSchema = (schema.properties !== undefined && Object.hasOwn(schema.properties, key) ? schema.properties[key] : undefined)
177181
?? patternProperties.find(([pattern]) => pattern.test(key))?.[1]
178182
?? schema.additionalProperties
179183

0 commit comments

Comments
 (0)