diff --git a/README.md b/README.md index bfd15b3..d3c27a0 100644 --- a/README.md +++ b/README.md @@ -2442,6 +2442,8 @@ The plugin automatically sanitizes tool parameters to prevent common attacks: - **Array length limits:** Unlimited by default - **Control character removal:** Strips null bytes and control characters - **Circular reference detection:** Prevents infinite loops +- **Prototype pollution protection:** Special key names (`__proto__`, `constructor`, `prototype`) are copied as plain own data properties, so untrusted input cannot change the prototype of the sanitized arguments +- **Key collision detection:** If stripping control characters maps two input keys onto the same output key, the request is refused instead of silently dropping one of the values The limits are configurable with the `toolArgumentLimits` option. Pass `Infinity` to disable one; Fastify's `bodyLimit` still caps the total request size. Arguments over a limit are refused with an diff --git a/src/security.ts b/src/security.ts index 1a3b00f..c085bbc 100644 --- a/src/security.ts +++ b/src/security.ts @@ -101,6 +101,13 @@ export function validateObjectDepth ( * once up front; every string reached afterwards, including object keys and * strings nested inside arrays at any depth, is length-checked and stripped of * control characters. + * + * Objects are copied by defining plain own data properties instead of direct + * assignment, so special key names such as `__proto__`, `constructor` and + * `prototype` cannot invoke inherited accessors and change the prototype of + * the sanitized result. When stripping control characters maps two different + * input keys onto the same output key, a `Duplicate key` error is thrown rather + * than silently dropping one of the values. */ export function sanitizeToolParams (params: Record, limits: ToolArgumentLimits = {}): Record { const maxStringLength = limits.maxStringLength ?? MAX_STRING_LENGTH @@ -123,7 +130,22 @@ export function sanitizeToolParams (params: Record, limits: ToolArg if (typeof value === 'object' && value !== null) { const sanitized: Record = {} for (const [key, item] of Object.entries(value)) { - sanitized[sanitizeString(key, maxStringLength)] = sanitizeValue(item) + const sanitizedKey = sanitizeString(key, maxStringLength) + // Copy with defineProperty rather than assignment: assignment on an + // ordinary object invokes inherited accessors for special names + // (notably `__proto__`), which lets untrusted input change the + // prototype of the sanitized result. defineProperty always creates a + // plain own data property and never triggers a setter, so `__proto__`, + // `constructor` and `prototype` are handled safely as ordinary keys. + if (Object.prototype.hasOwnProperty.call(sanitized, sanitizedKey)) { + throw new Error(`Duplicate key ${JSON.stringify(sanitizedKey)} after key sanitization`) + } + Object.defineProperty(sanitized, sanitizedKey, { + value: sanitizeValue(item), + enumerable: true, + writable: true, + configurable: true + }) } return sanitized } diff --git a/test/tool-argument-limits.test.ts b/test/tool-argument-limits.test.ts index 9cc94ee..527698e 100644 --- a/test/tool-argument-limits.test.ts +++ b/test/tool-argument-limits.test.ts @@ -195,4 +195,82 @@ describe('tool argument limits', () => { /depth of 3/ ) }) + + test('sanitizeToolParams does not let a direct __proto__ key change the result prototype', (t: TestContext) => { + const result = sanitizeToolParams(JSON.parse('{"__proto__": {"polluted": true}}')) + + t.assert.strictEqual(Object.getPrototypeOf(result), Object.prototype) + t.assert.strictEqual(Object.prototype.hasOwnProperty.call(result, '__proto__'), true) + const descriptor = Object.getOwnPropertyDescriptor(result, '__proto__') + t.assert.ok(descriptor) + t.assert.strictEqual(descriptor.value.polluted, true) + }) + + test('sanitizeToolParams does not let __proto__ pollute when nested in objects and arrays', (t: TestContext) => { + const result = sanitizeToolParams({ + obj: JSON.parse('{"__proto__": {"a": 1}}'), + xs: [JSON.parse('{"__proto__": {"b": 2}}')] + }) + + t.assert.strictEqual(Object.getPrototypeOf(result.obj), Object.prototype) + t.assert.strictEqual(Object.getPrototypeOf(result.xs[0]), Object.prototype) + t.assert.deepStrictEqual(Object.keys(result.obj), ['__proto__']) + t.assert.deepStrictEqual(Object.keys(result.xs[0]), ['__proto__']) + const objDescriptor = Object.getOwnPropertyDescriptor(result.obj, '__proto__') + const xsDescriptor = Object.getOwnPropertyDescriptor(result.xs[0], '__proto__') + t.assert.ok(objDescriptor) + t.assert.ok(xsDescriptor) + t.assert.strictEqual(objDescriptor.value.a, 1) + t.assert.strictEqual(xsDescriptor.value.b, 2) + }) + + test('sanitizeToolParams copies constructor and prototype keys as ordinary own properties', (t: TestContext) => { + const result = sanitizeToolParams(JSON.parse('{"constructor": 1, "prototype": 2}')) + + t.assert.strictEqual(Object.getPrototypeOf(result), Object.prototype) + t.assert.strictEqual(Object.prototype.hasOwnProperty.call(result, 'constructor'), true) + t.assert.strictEqual(Object.prototype.hasOwnProperty.call(result, 'prototype'), true) + t.assert.strictEqual(result.constructor, 1) + t.assert.strictEqual(result.prototype, 2) + }) + + test('sanitizeToolParams safely copies a key that sanitizes to a special property name', (t: TestContext) => { + // '__pro\u0000to__' strips the null character back to '__proto__' + const result = sanitizeToolParams({ '__pro\u0000to__': { polluted: true } }) + + t.assert.strictEqual(Object.getPrototypeOf(result), Object.prototype) + t.assert.strictEqual(Object.prototype.hasOwnProperty.call(result, '__proto__'), true) + const entries = Object.entries(result) + t.assert.strictEqual(entries.length, 1) + t.assert.strictEqual(entries[0][0], '__proto__') + t.assert.strictEqual((entries[0][1] as Record).polluted, true) + }) + + test('sanitizeToolParams detects when key sanitization maps two keys onto the same output key', (t: TestContext) => { + t.assert.throws( + () => sanitizeToolParams({ 'a\u0000b': 1, ab: 2 }), + /Duplicate key "ab" after key sanitization/ + ) + }) + + test('sanitizeToolParams detects key collisions nested inside objects and arrays', (t: TestContext) => { + t.assert.throws( + () => sanitizeToolParams({ o: { 'a\u0000b': 1, ab: 2 } }), + /Duplicate key "ab" after key sanitization/ + ) + t.assert.throws( + () => sanitizeToolParams({ xs: [{ 'a\u0000b': 1, ab: 2 }] }), + /Duplicate key "ab" after key sanitization/ + ) + }) + + test('refuses a tool call whose arguments collide after key sanitization', async (t: TestContext) => { + const { app, getInvocations } = await buildApp(t) + + const result = await callTool(app, { 'a\u0000b': 1, ab: 2 }) + + t.assert.strictEqual(result.isError, true) + t.assert.match((result.content[0] as { text: string }).text, /Duplicate key "ab" after key sanitization/) + t.assert.strictEqual(getInvocations(), 0) + }) })