From db2468de2a17ba0eee7f53a8b422f420f979bc99 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 12 Feb 2026 12:54:46 +0000 Subject: [PATCH 1/2] fix: harden headers iterable detection against polluted Object.prototype Treat headers as iterable only when Symbol.iterator is an own property, or when the object has a non-plain prototype with a function iterator.\n\nThis avoids misclassifying plain header objects as iterables when Object.prototype[Symbol.iterator] is polluted, which could otherwise cause silent header loss.\n\nApply the same hardening in request, redirect, and cache header handling, and add regression coverage for polluted Object.prototype[Symbol.iterator] while preserving support for legitimate iterable headers (e.g. Map/Headers). --- lib/core/request.js | 6 +++- lib/handler/redirect-handler.js | 6 +++- lib/util/cache.js | 33 +++++++++++--------- test/cache-interceptor/cache-utils.js | 44 ++++++++++++++++++++++++++ test/interceptors/redirect.js | 43 +++++++++++++++++++++++++ test/request.js | 45 +++++++++++++++++++++++++++ 6 files changed, 161 insertions(+), 16 deletions(-) create mode 100644 test/cache-interceptor/cache-utils.js diff --git a/lib/core/request.js b/lib/core/request.js index 7dbf781b4c4..802589137d7 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -174,7 +174,11 @@ class Request { processHeader(this, headers[i], headers[i + 1]) } } else if (headers && typeof headers === 'object') { - if (headers[Symbol.iterator]) { + const prototype = Object.getPrototypeOf(headers) + const ownIterator = Object.prototype.hasOwnProperty.call(headers, Symbol.iterator) + const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof headers[Symbol.iterator] === 'function') + + if (hasIterator) { for (const header of headers) { if (!Array.isArray(header) || header.length !== 2) { throw new InvalidArgumentError('headers must be in key-value pair format') diff --git a/lib/handler/redirect-handler.js b/lib/handler/redirect-handler.js index dd0f47170ae..726fe6732aa 100644 --- a/lib/handler/redirect-handler.js +++ b/lib/handler/redirect-handler.js @@ -222,7 +222,11 @@ function cleanRequestHeaders (headers, removeContent, unknownOrigin) { } } } else if (headers && typeof headers === 'object') { - const entries = typeof headers[Symbol.iterator] === 'function' ? headers : Object.entries(headers) + const prototype = Object.getPrototypeOf(headers) + const ownIterator = Object.prototype.hasOwnProperty.call(headers, Symbol.iterator) + const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof headers[Symbol.iterator] === 'function') + const entries = hasIterator ? headers : Object.entries(headers) + for (const [key, value] of entries) { if (!shouldRemoveHeader(key, removeContent, unknownOrigin)) { ret.push(key, value) diff --git a/lib/util/cache.js b/lib/util/cache.js index b7627d05e26..226b812c1c6 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -37,23 +37,28 @@ function normalizeHeaders (opts) { let headers if (opts.headers == null) { headers = {} - } else if (typeof opts.headers[Symbol.iterator] === 'function') { - headers = {} - for (const x of opts.headers) { - if (!Array.isArray(x)) { - throw new Error('opts.headers is not a valid header map') - } - const [key, val] = x - if (typeof key !== 'string' || typeof val !== 'string') { - throw new Error('opts.headers is not a valid header map') - } - headers[key.toLowerCase()] = val - } } else if (typeof opts.headers === 'object') { + const prototype = Object.getPrototypeOf(opts.headers) + const ownIterator = Object.prototype.hasOwnProperty.call(opts.headers, Symbol.iterator) + const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof opts.headers[Symbol.iterator] === 'function') + headers = {} - for (const key of Object.keys(opts.headers)) { - headers[key.toLowerCase()] = opts.headers[key] + if (hasIterator) { + for (const x of opts.headers) { + if (!Array.isArray(x)) { + throw new Error('opts.headers is not a valid header map') + } + const [key, val] = x + if (typeof key !== 'string' || typeof val !== 'string') { + throw new Error('opts.headers is not a valid header map') + } + headers[key.toLowerCase()] = val + } + } else { + for (const key of Object.keys(opts.headers)) { + headers[key.toLowerCase()] = opts.headers[key] + } } } else { throw new Error('opts.headers is not an object') diff --git a/test/cache-interceptor/cache-utils.js b/test/cache-interceptor/cache-utils.js new file mode 100644 index 00000000000..7b66e66d3aa --- /dev/null +++ b/test/cache-interceptor/cache-utils.js @@ -0,0 +1,44 @@ +'use strict' + +const { tspl } = require('@matteo.collina/tspl') +const { test } = require('node:test') +const { normalizeHeaders } = require('../../lib/util/cache') + +test('normalizeHeaders handles plain object headers with polluted Object.prototype[Symbol.iterator]', (t) => { + const { strictEqual } = tspl(t, { plan: 2 }) + + const originalIterator = Object.prototype[Symbol.iterator] + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = function * () {} + + try { + const headers = normalizeHeaders({ + headers: { + Authorization: 'Bearer token', + 'X-Test': 'ok' + } + }) + + strictEqual(headers.authorization, 'Bearer token') + strictEqual(headers['x-test'], 'ok') + } finally { + if (originalIterator === undefined) { + delete Object.prototype[Symbol.iterator] + } else { + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = originalIterator + } + } +}) + +test('normalizeHeaders handles headers from Map', (t) => { + const { strictEqual } = tspl(t, { plan: 1 }) + + const headers = normalizeHeaders({ + headers: new Map([ + ['X-Test', 'ok'] + ]) + }) + + strictEqual(headers['x-test'], 'ok') +}) diff --git a/test/interceptors/redirect.js b/test/interceptors/redirect.js index 59db5d3d90c..99acfbfa80d 100644 --- a/test/interceptors/redirect.js +++ b/test/interceptors/redirect.js @@ -749,6 +749,49 @@ test('should redirect to relative URL according to RFC 7231', async t => { t.strictEqual(finalPath, '/absolute/b') }) +test('same-origin redirect preserves plain object headers with polluted Object.prototype[Symbol.iterator]', async (t) => { + const { strictEqual } = tspl(t, { plan: 2 }) + + const server = createServer((req, res) => { + if (req.url === '/redirect') { + res.writeHead(302, { + Location: '/final' + }) + res.end() + return + } + + strictEqual(req.headers['x-custom'], 'ok') + res.end('redirected') + }).listen(0) + + const originalIterator = Object.prototype[Symbol.iterator] + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = function * () {} + + try { + await once(server, 'listening') + + const res = await undici.request(`http://localhost:${server.address().port}/redirect`, { + dispatcher: new undici.Agent({}).compose(redirect({ maxRedirections: 1 })), + headers: { + 'X-Custom': 'ok' + } + }) + + const text = await res.body.text() + strictEqual(text, 'redirected') + } finally { + if (originalIterator === undefined) { + delete Object.prototype[Symbol.iterator] + } else { + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = originalIterator + } + server.close() + } +}) + test('Cross-origin redirects clear forbidden headers', async (t) => { const { strictEqual } = tspl(t, { plan: 6 }) diff --git a/test/request.js b/test/request.js index f0841654a7a..83d9cafe1c2 100644 --- a/test/request.js +++ b/test/request.js @@ -432,6 +432,51 @@ describe('Should include headers from iterable objects', scope => { }) }) + test('Should include headers from plain objects with polluted Object.prototype[Symbol.iterator]', async t => { + t = tspl(t, { plan: 3 }) + + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + t.strictEqual('GET', req.method) + t.strictEqual(`localhost:${server.address().port}`, req.headers.host) + t.strictEqual(req.headers.hello, 'world') + res.statusCode = 200 + res.end('hello') + }) + + const headers = { + hello: 'world' + } + + const originalIterator = Object.prototype[Symbol.iterator] + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = function * () {} + + try { + await new Promise((resolve, reject) => { + server.listen(0, (err) => { + if (err != null) reject(err) + else resolve() + }) + }) + + await request({ + method: 'GET', + origin: `http://localhost:${server.address().port}`, + reset: true, + headers + }) + } finally { + if (originalIterator === undefined) { + delete Object.prototype[Symbol.iterator] + } else { + // eslint-disable-next-line no-extend-native + Object.prototype[Symbol.iterator] = originalIterator + } + server.closeAllConnections?.() + server.close() + } + }) + test('Should throw error if headers iterable object does not yield key-value pairs', async t => { t = tspl(t, { plan: 2 }) From ee06c38e3b469f9d22192b776d3f4e0523ad22f5 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 2 Mar 2026 16:30:56 +0100 Subject: [PATCH 2/2] refactor: extract hasSafeIterator utility to deduplicate iterator checks Co-Authored-By: Claude Opus 4.6 Signed-off-by: Matteo Collina --- lib/core/request.js | 7 ++----- lib/core/util.js | 15 +++++++++++++++ lib/handler/redirect-handler.js | 5 +---- lib/util/cache.js | 9 +++------ 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/core/request.js b/lib/core/request.js index 802589137d7..2b1ccdf638f 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -13,6 +13,7 @@ const { isBuffer, isFormDataLike, isIterable, + hasSafeIterator, isBlobLike, serializePathWithQuery, assertRequestHandler, @@ -174,11 +175,7 @@ class Request { processHeader(this, headers[i], headers[i + 1]) } } else if (headers && typeof headers === 'object') { - const prototype = Object.getPrototypeOf(headers) - const ownIterator = Object.prototype.hasOwnProperty.call(headers, Symbol.iterator) - const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof headers[Symbol.iterator] === 'function') - - if (hasIterator) { + if (hasSafeIterator(headers)) { for (const header of headers) { if (!Array.isArray(header) || header.length !== 2) { throw new InvalidArgumentError('headers must be in key-value pair format') diff --git a/lib/core/util.js b/lib/core/util.js index be2c1a7320d..db8dda53a81 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -327,6 +327,20 @@ function isIterable (obj) { return !!(obj != null && (typeof obj[Symbol.iterator] === 'function' || typeof obj[Symbol.asyncIterator] === 'function')) } +/** + * Checks whether an object has a safe Symbol.iterator — i.e. one that is + * either own or inherited from a non-Object.prototype chain. This prevents + * prototype-pollution attacks from injecting a fake iterator on + * Object.prototype. + * @param {object} obj + * @returns {boolean} + */ +function hasSafeIterator (obj) { + const prototype = Object.getPrototypeOf(obj) + const ownIterator = Object.prototype.hasOwnProperty.call(obj, Symbol.iterator) + return ownIterator || (prototype != null && prototype !== Object.prototype && typeof obj[Symbol.iterator] === 'function') +} + /** * @param {Blob|Buffer|import ('stream').Stream} body * @returns {number|null} @@ -918,6 +932,7 @@ module.exports = { getServerName, isStream, isIterable, + hasSafeIterator, isAsyncIterable, isDestroyed, headerNameToString, diff --git a/lib/handler/redirect-handler.js b/lib/handler/redirect-handler.js index 726fe6732aa..d088d5488af 100644 --- a/lib/handler/redirect-handler.js +++ b/lib/handler/redirect-handler.js @@ -222,10 +222,7 @@ function cleanRequestHeaders (headers, removeContent, unknownOrigin) { } } } else if (headers && typeof headers === 'object') { - const prototype = Object.getPrototypeOf(headers) - const ownIterator = Object.prototype.hasOwnProperty.call(headers, Symbol.iterator) - const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof headers[Symbol.iterator] === 'function') - const entries = hasIterator ? headers : Object.entries(headers) + const entries = util.hasSafeIterator(headers) ? headers : Object.entries(headers) for (const [key, value] of entries) { if (!shouldRemoveHeader(key, removeContent, unknownOrigin)) { diff --git a/lib/util/cache.js b/lib/util/cache.js index 226b812c1c6..5968f4efe5f 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -2,7 +2,8 @@ const { safeHTTPMethods, - pathHasQueryOrFragment + pathHasQueryOrFragment, + hasSafeIterator } = require('../core/util') const { serializePathWithQuery } = require('../core/util') @@ -38,13 +39,9 @@ function normalizeHeaders (opts) { if (opts.headers == null) { headers = {} } else if (typeof opts.headers === 'object') { - const prototype = Object.getPrototypeOf(opts.headers) - const ownIterator = Object.prototype.hasOwnProperty.call(opts.headers, Symbol.iterator) - const hasIterator = ownIterator || (prototype != null && prototype !== Object.prototype && typeof opts.headers[Symbol.iterator] === 'function') - headers = {} - if (hasIterator) { + if (hasSafeIterator(opts.headers)) { for (const x of opts.headers) { if (!Array.isArray(x)) { throw new Error('opts.headers is not a valid header map')