From 64e3402da4e032e68de46acb52800c9a06aaea3f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 2 Apr 2024 12:41:40 +0200 Subject: [PATCH] Merge pull request from GHSA-m4v8-wqvr-p9f7 Signed-off-by: Matteo Collina --- lib/core/constants.js | 118 +++++++++++++++++++++++++++ lib/core/util.js | 11 +++ lib/handler/RedirectHandler.js | 17 ++-- test/redirect-cross-origin-header.js | 51 ++++++++++++ 4 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 lib/core/constants.js create mode 100644 test/redirect-cross-origin-header.js diff --git a/lib/core/constants.js b/lib/core/constants.js new file mode 100644 index 00000000000..6ec770dd533 --- /dev/null +++ b/lib/core/constants.js @@ -0,0 +1,118 @@ +'use strict' + +/** @type {Record} */ +const headerNameLowerCasedRecord = {} + +// https://developer.mozilla.org/docs/Web/HTTP/Headers +const wellknownHeaderNames = [ + 'Accept', + 'Accept-Encoding', + 'Accept-Language', + 'Accept-Ranges', + 'Access-Control-Allow-Credentials', + 'Access-Control-Allow-Headers', + 'Access-Control-Allow-Methods', + 'Access-Control-Allow-Origin', + 'Access-Control-Expose-Headers', + 'Access-Control-Max-Age', + 'Access-Control-Request-Headers', + 'Access-Control-Request-Method', + 'Age', + 'Allow', + 'Alt-Svc', + 'Alt-Used', + 'Authorization', + 'Cache-Control', + 'Clear-Site-Data', + 'Connection', + 'Content-Disposition', + 'Content-Encoding', + 'Content-Language', + 'Content-Length', + 'Content-Location', + 'Content-Range', + 'Content-Security-Policy', + 'Content-Security-Policy-Report-Only', + 'Content-Type', + 'Cookie', + 'Cross-Origin-Embedder-Policy', + 'Cross-Origin-Opener-Policy', + 'Cross-Origin-Resource-Policy', + 'Date', + 'Device-Memory', + 'Downlink', + 'ECT', + 'ETag', + 'Expect', + 'Expect-CT', + 'Expires', + 'Forwarded', + 'From', + 'Host', + 'If-Match', + 'If-Modified-Since', + 'If-None-Match', + 'If-Range', + 'If-Unmodified-Since', + 'Keep-Alive', + 'Last-Modified', + 'Link', + 'Location', + 'Max-Forwards', + 'Origin', + 'Permissions-Policy', + 'Pragma', + 'Proxy-Authenticate', + 'Proxy-Authorization', + 'RTT', + 'Range', + 'Referer', + 'Referrer-Policy', + 'Refresh', + 'Retry-After', + 'Sec-WebSocket-Accept', + 'Sec-WebSocket-Extensions', + 'Sec-WebSocket-Key', + 'Sec-WebSocket-Protocol', + 'Sec-WebSocket-Version', + 'Server', + 'Server-Timing', + 'Service-Worker-Allowed', + 'Service-Worker-Navigation-Preload', + 'Set-Cookie', + 'SourceMap', + 'Strict-Transport-Security', + 'Supports-Loading-Mode', + 'TE', + 'Timing-Allow-Origin', + 'Trailer', + 'Transfer-Encoding', + 'Upgrade', + 'Upgrade-Insecure-Requests', + 'User-Agent', + 'Vary', + 'Via', + 'WWW-Authenticate', + 'X-Content-Type-Options', + 'X-DNS-Prefetch-Control', + 'X-Frame-Options', + 'X-Permitted-Cross-Domain-Policies', + 'X-Powered-By', + 'X-Requested-With', + 'X-XSS-Protection' +] + +for (let i = 0; i < wellknownHeaderNames.length; ++i) { + const key = wellknownHeaderNames[i] + const lowerCasedKey = key.toLowerCase() + headerNameLowerCasedRecord[key] = headerNameLowerCasedRecord[lowerCasedKey] = + lowerCasedKey +} + +// Note: object prototypes should not be able to be referenced. e.g. `Object#hasOwnProperty`. +Object.setPrototypeOf(headerNameLowerCasedRecord, null) + +module.exports = { + wellknownHeaderNames, + headerNameLowerCasedRecord +} diff --git a/lib/core/util.js b/lib/core/util.js index 8d5450ba0c0..5741650ea2e 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -9,6 +9,7 @@ const { InvalidArgumentError } = require('./errors') const { Blob } = require('buffer') const nodeUtil = require('util') const { stringify } = require('querystring') +const { headerNameLowerCasedRecord } = require('./constants') const [nodeMajor, nodeMinor] = process.versions.node.split('.').map(v => Number(v)) @@ -218,6 +219,15 @@ function parseKeepAliveTimeout (val) { return m ? parseInt(m[1], 10) * 1000 : null } +/** + * Retrieves a header name and returns its lowercase value. + * @param {string | Buffer} value Header name + * @returns {string} + */ +function headerNameToString (value) { + return headerNameLowerCasedRecord[value] || value.toLowerCase() +} + function parseHeaders (headers, obj = {}) { // For H2 support if (!Array.isArray(headers)) return headers @@ -489,6 +499,7 @@ module.exports = { isIterable, isAsyncIterable, isDestroyed, + headerNameToString, parseRawHeaders, parseHeaders, parseKeepAliveTimeout, diff --git a/lib/handler/RedirectHandler.js b/lib/handler/RedirectHandler.js index baca27ed147..933725fbd01 100644 --- a/lib/handler/RedirectHandler.js +++ b/lib/handler/RedirectHandler.js @@ -184,12 +184,17 @@ function parseLocation (statusCode, headers) { // https://tools.ietf.org/html/rfc7231#section-6.4.4 function shouldRemoveHeader (header, removeContent, unknownOrigin) { - return ( - (header.length === 4 && header.toString().toLowerCase() === 'host') || - (removeContent && header.toString().toLowerCase().indexOf('content-') === 0) || - (unknownOrigin && header.length === 13 && header.toString().toLowerCase() === 'authorization') || - (unknownOrigin && header.length === 6 && header.toString().toLowerCase() === 'cookie') - ) + if (header.length === 4) { + return util.headerNameToString(header) === 'host' + } + if (removeContent && util.headerNameToString(header).startsWith('content-')) { + return true + } + if (unknownOrigin && (header.length === 13 || header.length === 6 || header.length === 19)) { + const name = util.headerNameToString(header) + return name === 'authorization' || name === 'cookie' || name === 'proxy-authorization' + } + return false } // https://tools.ietf.org/html/rfc7231#section-6.4 diff --git a/test/redirect-cross-origin-header.js b/test/redirect-cross-origin-header.js new file mode 100644 index 00000000000..8313e00c019 --- /dev/null +++ b/test/redirect-cross-origin-header.js @@ -0,0 +1,51 @@ +'use strict' + +const { test } = require('tap') +const { createServer } = require('node:http') +const { once } = require('node:events') +const { request } = require('..') + +test('Cross-origin redirects clear forbidden headers', async (t) => { + t.plan(6) + + const server1 = createServer((req, res) => { + t.equal(req.headers.cookie, undefined) + t.equal(req.headers.authorization, undefined) + t.equal(req.headers['proxy-authorization'], undefined) + + res.end('redirected') + }).listen(0) + + const server2 = createServer((req, res) => { + t.equal(req.headers.authorization, 'test') + t.equal(req.headers.cookie, 'ddd=dddd') + + res.writeHead(302, { + ...req.headers, + Location: `http://localhost:${server1.address().port}` + }) + res.end() + }).listen(0) + + t.teardown(() => { + server1.close() + server2.close() + }) + + await Promise.all([ + once(server1, 'listening'), + once(server2, 'listening') + ]) + + const res = await request(`http://localhost:${server2.address().port}`, { + maxRedirections: 1, + headers: { + Authorization: 'test', + Cookie: 'ddd=dddd', + 'Proxy-Authorization': 'test' + } + }) + + const text = await res.body.text() + t.equal(text, 'redirected') +})