Skip to content

Commit

Permalink
fix(form-core): prevent prototype pollution and update Remix dependen…
Browse files Browse the repository at this point in the history
  • Loading branch information
moehaje authored Feb 20, 2025
1 parent 9dea996 commit 455522c
Show file tree
Hide file tree
Showing 7 changed files with 801 additions and 368 deletions.
2 changes: 1 addition & 1 deletion docs/reference/functions/mergeform.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: mergeForm
function mergeForm<TFormData, TFormValidator>(baseForm, state): FormApi<NoInfer<TFormData>, NoInfer<TFormValidator>>
```

Defined in: [packages/form-core/src/mergeForm.ts:36](https://github.com/TanStack/form/blob/main/packages/form-core/src/mergeForm.ts#L36)
Defined in: [packages/form-core/src/mergeForm.ts:75](https://github.com/TanStack/form/blob/main/packages/form-core/src/mergeForm.ts#L75)

## Type Parameters

Expand Down
2 changes: 1 addition & 1 deletion examples/react/remix/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"_test:types": "tsc"
},
"dependencies": {
"@remix-run/node": "^2.15.0",
"@remix-run/node": "^2.15.3",
"@remix-run/react": "^2.15.0",
"@remix-run/serve": "^2.15.0",
"@tanstack/react-form": "^0.42.0",
Expand Down
77 changes: 58 additions & 19 deletions packages/form-core/src/mergeForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,73 @@ import type { FormApi } from './FormApi'
import type { Validator } from './types'
import type { NoInfer } from './util-types'

function isValidKey(key: string | number | symbol): boolean {
const dangerousProps = ['__proto__', 'constructor', 'prototype']
return !dangerousProps.includes(String(key))
}

/**
* @private
*/
export function mutateMergeDeep(target: object, source: object): object {
export function mutateMergeDeep(
target: object | null | undefined,
source: object | null | undefined,
): object {
// Early return if either is not an object
if (target === null || target === undefined || typeof target !== 'object')
return {} as object
if (source === null || source === undefined || typeof source !== 'object')
return target

const targetKeys = Object.keys(target)
const sourceKeys = Object.keys(source)
const keySet = new Set([...targetKeys, ...sourceKeys])

for (const key of keySet) {
const targetKey = key as never as keyof typeof target
const sourceKey = key as never as keyof typeof source

if (Array.isArray(target[targetKey]) && Array.isArray(source[sourceKey])) {
// always use the source array to prevent array fields from multiplying
target[targetKey] = source[sourceKey] as [] as never
} else if (
typeof target[targetKey] === 'object' &&
typeof source[sourceKey] === 'object'
) {
mutateMergeDeep(target[targetKey] as {}, source[sourceKey] as {})
} else {
// Prevent assigning undefined to target, only if undefined is not explicitly set on source
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!(sourceKey in source) && source[sourceKey] === undefined) {
continue
}
target[targetKey] = source[sourceKey] as never
if (!isValidKey(key)) continue

const targetKey = key as keyof typeof target
const sourceKey = key as keyof typeof source

if (!Object.hasOwn(source, sourceKey)) continue

const sourceValue = source[sourceKey] as unknown
const targetValue = target[targetKey] as unknown

// Handle arrays
if (Array.isArray(targetValue) && Array.isArray(sourceValue)) {
Object.defineProperty(target, key, {
value: [...sourceValue],
enumerable: true,
writable: true,
configurable: true,
})
continue
}

// Handle nested objects (type assertion to satisfy ESLint)
const isTargetObj = typeof targetValue === 'object' && targetValue !== null
const isSourceObj = typeof sourceValue === 'object' && sourceValue !== null
const areObjects =
isTargetObj &&
isSourceObj &&
!Array.isArray(targetValue) &&
!Array.isArray(sourceValue)

if (areObjects) {
mutateMergeDeep(targetValue as object, sourceValue as object)
continue
}

// Handle all other cases
Object.defineProperty(target, key, {
value: sourceValue,
enumerable: true,
writable: true,
configurable: true,
})
}

return target
}

Expand Down
100 changes: 100 additions & 0 deletions packages/form-core/tests/mergeForm.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { describe, expect, it } from 'vitest'
import { mutateMergeDeep } from '../src/mergeForm'

type TestObject = Record<string, any>

describe('mutateMergeDeep', () => {
it('should prevent prototype pollution through __proto__', () => {
const target: TestObject = {}
const malicious = {
__proto__: {
polluted: true,
},
}

mutateMergeDeep(target, malicious)
expect(({} as TestObject).polluted).toBeUndefined()
expect((Object.prototype as TestObject).polluted).toBeUndefined()
})

it('should prevent prototype pollution through constructor', () => {
const target: TestObject = {}
const malicious = {
constructor: {
prototype: {
polluted: true,
},
},
}

mutateMergeDeep(target, malicious)
expect(({} as TestObject).polluted).toBeUndefined()
})

it('should handle null values correctly', () => {
const target = { details: null }
const source = { details: { age: 25 } }

mutateMergeDeep(target, source)
expect(target).toStrictEqual({ details: { age: 25 } })
})

it('should preserve object references when updating nested objects', () => {
const target: { user: { details: TestObject } } = { user: { details: {} } }
const source = { user: { details: { name: 'test' } } }

const originalDetails = target.user.details
mutateMergeDeep(target, source)
expect(target.user.details).toBe(originalDetails)
expect(target.user.details.name).toBe('test')
})

it('Should merge two objects by mutating', () => {
const a = { a: 1 }
const b = { b: 2 }
mutateMergeDeep(a, b)
expect(a).toStrictEqual({ a: 1, b: 2 })
})

it('Should merge two objects including overwriting with undefined', () => {
const a = { a: 1 }
const b = { a: undefined }
mutateMergeDeep(a, b)
expect(a).toStrictEqual({ a: undefined })
})

it('Should merge two object by overriding arrays', () => {
const target = { a: [1] }
const source = { a: [2] }
mutateMergeDeep(target, source)
expect(target).toStrictEqual({ a: [2] })
})

it('Should merge add array element when it does not exist in target', () => {
const target = { a: [] }
const source = { a: [2] }
mutateMergeDeep(target, source)
expect(target).toStrictEqual({ a: [2] })
})

it('Should override the target array if source is undefined', () => {
const target = { a: [2] }
const source = { a: undefined }
mutateMergeDeep(target, source)
expect(target).toStrictEqual({ a: undefined })
})

it('Should merge update array element when it does not exist in source', () => {
const target = { a: [2] }
const source = { a: [] }
mutateMergeDeep(target, source)
expect(target).toStrictEqual({ a: [] })
})

it('Should merge two deeply nested objects', () => {
const a = { a: { a: 1 } }
const b = { a: { b: 2 } }
mutateMergeDeep(a, b)
expect(a).toStrictEqual({ a: { a: 1, b: 2 } })
})
})
53 changes: 0 additions & 53 deletions packages/form-core/tests/mutateMergeDeep.spec.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react-form/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"src"
],
"dependencies": {
"@remix-run/node": "^2.15.0",
"@remix-run/node": "^2.15.3",
"@tanstack/form-core": "workspace:*",
"@tanstack/react-store": "^0.7.0",
"decode-formdata": "^0.8.0"
Expand Down
Loading

0 comments on commit 455522c

Please sign in to comment.