fix: prevent prototype pollution via __proto__ in FormData field names

.github/workflows/validate.yml

@@ -16,7 +16,7 @@ jobs:
     if: ${{ !contains(github.head_ref, 'all-contributors') }}
     strategy:
       matrix:
-        node: [12, 14, 16]
+        node: [14, 16]
     runs-on: ubuntu-latest
     steps:
       - name: 🛑 Cancel Previous Runs

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+  "js/ts.tsdk.path": "node_modules/typescript/lib"
+}

SECURITY.md

@@ -0,0 +1,41 @@
+# Security Policy
+
+## Reporting a vulnerability
+
+If you believe you've found a security vulnerability in
+`parse-nested-form-data`, please report it privately. **Do not open a public
+issue.**
+
+Preferred channel:
+[open a private vulnerability report](https://github.com/milamer/parse-nested-form-data/security/advisories/new)
+on this repository. GitHub's private reporting flow keeps the discussion private
+until a fix is ready and supports CVE assignment.
+
+If you cannot use GitHub's private reporting, email **chris@schurr.dev** with
+`[security] parse-nested-form-data` in the subject line.
+
+When reporting, please include:
+
+- A description of the issue and its impact
+- Affected version(s)
+- A minimal reproduction (PoC code, input that triggers the bug, expected vs.
+  actual behavior)
+- Any suggested mitigation, if you have one
+
+You will receive an acknowledgement within a few business days. I'll keep you
+updated as the fix progresses and credit you in the published advisory unless
+you'd prefer to remain anonymous.
+
+## Disclosure
+
+Coordinated disclosure is preferred. The default window is 90 days from initial
+report to public disclosure, which can be shortened if a fix ships sooner or
+extended by mutual agreement.
+
+Once a patched release is available on npm, the corresponding GitHub Security
+Advisory is published so that downstream users are notified through Dependabot
+and `npm audit`.
+
+## Supported versions
+
+Only the latest published version on npm receives security fixes.

package.json

@@ -44,6 +44,7 @@
   "devDependencies": {
     "@remix-run/web-file": "^3.0.2",
     "@remix-run/web-form-data": "^3.0.3",
+    "@types/minimatch": "^3.0.5",
     "kcd-scripts": "^12.3.0",
     "rimraf": "^3.0.2",
     "typescript": "^4.8.4"

src/__tests__/index.ts

@@ -1,6 +1,11 @@
 import {FormData} from '@remix-run/web-form-data'
 import {File} from '@remix-run/web-file'
-import {DuplicateKeyError, MixedArrayError, parseFormData} from '../'
+import {
+  DuplicateKeyError,
+  ForbiddenKeyError,
+  MixedArrayError,
+  parseFormData,
+} from '../'
 
 describe('basic functionality', () => {
   describe('transform value', () => {
@@ -409,3 +414,80 @@ describe('complex examples', () => {
     })
   })
 })
+
+describe('prototype pollution protection', () => {
+  const proto = Object.prototype as unknown as {[key: string]: unknown}
+  afterEach(() => {
+    // Defensive: clean any test-leaked properties off Object.prototype so a
+    // failure can't silently affect later tests.
+    delete proto.polluted
+  })
+
+  it('rejects `__proto__` as a top-level key', () => {
+    const formData = new FormData()
+    formData.append('__proto__.polluted', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('__proto__'),
+    )
+    expect(proto.polluted).toBeUndefined()
+  })
+
+  it('rejects `__proto__` as a nested key', () => {
+    const formData = new FormData()
+    formData.append('a.__proto__.polluted', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('a.__proto__'),
+    )
+    expect(proto.polluted).toBeUndefined()
+  })
+
+  it('rejects `__proto__` reached through an array element', () => {
+    const formData = new FormData()
+    formData.append('a[0].__proto__.polluted', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('a[0].__proto__'),
+    )
+    expect(proto.polluted).toBeUndefined()
+  })
+
+  it('rejects `constructor` as a key', () => {
+    const formData = new FormData()
+    formData.append('constructor.prototype.polluted', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('constructor'),
+    )
+  })
+
+  it('rejects `prototype` as a key', () => {
+    const formData = new FormData()
+    formData.append('prototype.polluted', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('prototype'),
+    )
+  })
+
+  it('rejects `__proto__` as a leaf assignment', () => {
+    const formData = new FormData()
+    formData.append('a.__proto__', 'yes')
+    expect(() => parseFormData(formData)).toThrowError(
+      new ForbiddenKeyError('a.__proto__'),
+    )
+  })
+
+  // The cases below pin the invariant for the array branch. Today the regex
+  // restricts array-index segments to digit characters, so a forbidden key
+  // can't reach the array branch (the cases parse as an object pathPart with
+  // a trailing `]` and trip the array/object duplicate-key guard). If the
+  // regex is ever loosened to accept arbitrary content inside `[...]`, these
+  // tests will start failing and the array branch will need its own
+  // forbidden-key check.
+  it.each(['__proto__', 'constructor', 'prototype'])(
+    'does not pollute via `a[%s]`',
+    key => {
+      const formData = new FormData()
+      formData.append(`a[${key}]`, 'yes')
+      expect(() => parseFormData(formData)).toThrow()
+      expect(proto.polluted).toBeUndefined()
+    },
+  )
+})

src/index.ts

@@ -73,6 +73,29 @@ export class MixedArrayError extends Error {
     this.key = key
   }
 }
+/**
+ * Thrown when a path part would access or assign a property on
+ * `Object.prototype` (`__proto__`, `constructor`, `prototype`). Rejected
+ * regardless of whether pollution would actually occur, to keep input
+ * unambiguous.
+ *
+ * @example
+ * ```ts
+ * const formData = new FormData()
+ * formData.append('__proto__.polluted', 'yes')
+ * parseFormData(formData)
+ * // throws ForbiddenKeyError('__proto__')
+ * ```
+ */
+export class ForbiddenKeyError extends Error {
+  key: string
+  constructor(key: string) {
+    super(`Forbidden key at path part ${key}`)
+    this.key = key
+  }
+}
+
+const FORBIDDEN_OBJECT_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
 
 type JsonObject = {[Key in string]?: JsonValue}
 type JsonArray = Array<JsonValue>
@@ -333,6 +356,9 @@ function handlePathPart(
   nextPathValue: JsonValue | undefined,
   setNextPathValue: (value: JsonValue) => void,
 ] {
+  if (FORBIDDEN_OBJECT_KEYS.has(pathPart.path)) {
+    throw new ForbiddenKeyError(pathPart.pathToPart)
+  }
   if (pathPart.type === 'object') {
     if (Array.isArray(currentPathObject)) {
       throw new DuplicateKeyError(pathPart.pathToPart)