Skip to content

Commit 83264b3

Browse files
author
Christian Schurr
committed
fix: prevent prototype pollution via __proto__ in FormData field names
`parseFormData` walked bracket and dot-notation field names into nested objects without filtering reserved property keys. A single field whose name began with `__proto__` (or contained `.__proto__.` mid-path) caused the parser to traverse and assign onto `Object.prototype`, polluting the prototype chain of every plain object in the running process. `handlePathPart` now throws a new `ForbiddenKeyError` (also exported) when an object-type path segment is `__proto__`, `constructor`, or `prototype`. The array branch is unaffected today - the regex restricts array-index segments to digits only - and three new tests pin that invariant so a future regex change would surface the gap. Reported responsibly by Mohamed Bassia (https://github.com/0xBassia).
1 parent b2e8fc2 commit 83264b3

3 files changed

Lines changed: 112 additions & 1 deletion

File tree

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"js/ts.tsdk.path": "node_modules/typescript/lib"
3+
}

src/__tests__/index.ts

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import {FormData} from '@remix-run/web-form-data'
22
import {File} from '@remix-run/web-file'
3-
import {DuplicateKeyError, MixedArrayError, parseFormData} from '../'
3+
import {
4+
DuplicateKeyError,
5+
ForbiddenKeyError,
6+
MixedArrayError,
7+
parseFormData,
8+
} from '../'
49

510
describe('basic functionality', () => {
611
describe('transform value', () => {
@@ -409,3 +414,80 @@ describe('complex examples', () => {
409414
})
410415
})
411416
})
417+
418+
describe('prototype pollution protection', () => {
419+
const proto = Object.prototype as unknown as {[key: string]: unknown}
420+
afterEach(() => {
421+
// Defensive: clean any test-leaked properties off Object.prototype so a
422+
// failure can't silently affect later tests.
423+
delete proto.polluted
424+
})
425+
426+
it('rejects `__proto__` as a top-level key', () => {
427+
const formData = new FormData()
428+
formData.append('__proto__.polluted', 'yes')
429+
expect(() => parseFormData(formData)).toThrowError(
430+
new ForbiddenKeyError('__proto__'),
431+
)
432+
expect(proto.polluted).toBeUndefined()
433+
})
434+
435+
it('rejects `__proto__` as a nested key', () => {
436+
const formData = new FormData()
437+
formData.append('a.__proto__.polluted', 'yes')
438+
expect(() => parseFormData(formData)).toThrowError(
439+
new ForbiddenKeyError('a.__proto__'),
440+
)
441+
expect(proto.polluted).toBeUndefined()
442+
})
443+
444+
it('rejects `__proto__` reached through an array element', () => {
445+
const formData = new FormData()
446+
formData.append('a[0].__proto__.polluted', 'yes')
447+
expect(() => parseFormData(formData)).toThrowError(
448+
new ForbiddenKeyError('a[0].__proto__'),
449+
)
450+
expect(proto.polluted).toBeUndefined()
451+
})
452+
453+
it('rejects `constructor` as a key', () => {
454+
const formData = new FormData()
455+
formData.append('constructor.prototype.polluted', 'yes')
456+
expect(() => parseFormData(formData)).toThrowError(
457+
new ForbiddenKeyError('constructor'),
458+
)
459+
})
460+
461+
it('rejects `prototype` as a key', () => {
462+
const formData = new FormData()
463+
formData.append('prototype.polluted', 'yes')
464+
expect(() => parseFormData(formData)).toThrowError(
465+
new ForbiddenKeyError('prototype'),
466+
)
467+
})
468+
469+
it('rejects `__proto__` as a leaf assignment', () => {
470+
const formData = new FormData()
471+
formData.append('a.__proto__', 'yes')
472+
expect(() => parseFormData(formData)).toThrowError(
473+
new ForbiddenKeyError('a.__proto__'),
474+
)
475+
})
476+
477+
// The cases below pin the invariant for the array branch. Today the regex
478+
// restricts array-index segments to digit characters, so a forbidden key
479+
// can't reach the array branch (the cases parse as an object pathPart with
480+
// a trailing `]` and trip the array/object duplicate-key guard). If the
481+
// regex is ever loosened to accept arbitrary content inside `[...]`, these
482+
// tests will start failing and the array branch will need its own
483+
// forbidden-key check.
484+
it.each(['__proto__', 'constructor', 'prototype'])(
485+
'does not pollute via `a[%s]`',
486+
key => {
487+
const formData = new FormData()
488+
formData.append(`a[${key}]`, 'yes')
489+
expect(() => parseFormData(formData)).toThrow()
490+
expect(proto.polluted).toBeUndefined()
491+
},
492+
)
493+
})

src/index.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,29 @@ export class MixedArrayError extends Error {
7373
this.key = key
7474
}
7575
}
76+
/**
77+
* Thrown when a path part would access or assign a property on
78+
* `Object.prototype` (`__proto__`, `constructor`, `prototype`). Rejected
79+
* regardless of whether pollution would actually occur, to keep input
80+
* unambiguous.
81+
*
82+
* @example
83+
* ```ts
84+
* const formData = new FormData()
85+
* formData.append('__proto__.polluted', 'yes')
86+
* parseFormData(formData)
87+
* // throws ForbiddenKeyError('__proto__')
88+
* ```
89+
*/
90+
export class ForbiddenKeyError extends Error {
91+
key: string
92+
constructor(key: string) {
93+
super(`Forbidden key at path part ${key}`)
94+
this.key = key
95+
}
96+
}
97+
98+
const FORBIDDEN_OBJECT_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
7699

77100
type JsonObject = {[Key in string]?: JsonValue}
78101
type JsonArray = Array<JsonValue>
@@ -333,6 +356,9 @@ function handlePathPart(
333356
nextPathValue: JsonValue | undefined,
334357
setNextPathValue: (value: JsonValue) => void,
335358
] {
359+
if (FORBIDDEN_OBJECT_KEYS.has(pathPart.path)) {
360+
throw new ForbiddenKeyError(pathPart.pathToPart)
361+
}
336362
if (pathPart.type === 'object') {
337363
if (Array.isArray(currentPathObject)) {
338364
throw new DuplicateKeyError(pathPart.pathToPart)

0 commit comments

Comments
 (0)