From 9d92bcd32639d1eea5b89f03ae45f248d3bb058e Mon Sep 17 00:00:00 2001 From: Jay Date: Sat, 2 May 2026 12:40:59 +0200 Subject: [PATCH] fix: gadgets and smaller issues (#10833) * chore: remove un-needed ghsa in the comments of files * fix: auth header * fix: escape regex chars in cookies.read * fix: read-side merge and descriptors * fix: enable redaction in the .toJson for errors * fix: general IPv4-mapped IPv6 normalization in NO_PROXY * fix: added regression tests for scenarios already covered * chore: remove un-needed comments * fix: harden proxy host detection and error redaction * fix: make form-data header change opt-in * fix: apply suggestions form github review * fix: cubic review * fix: widen the regexs for matches Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix: smaller issue found by cubic * fix: address prototype chain * fix: update as per cubic --------- Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- README.md | 22 + docs/pages/advanced/error-handling.md | 11 + docs/pages/advanced/request-config.md | 24 + index.d.cts | 2 + index.d.ts | 2 + lib/adapters/adapters.js | 6 +- lib/adapters/http.js | 98 +++- lib/core/AxiosError.js | 86 ++- lib/core/AxiosHeaders.js | 3 + lib/core/mergeConfig.js | 9 +- lib/helpers/cookies.js | 16 +- lib/helpers/resolveConfig.js | 45 +- lib/helpers/shouldBypassProxy.js | 27 +- lib/helpers/validator.js | 2 +- lib/utils.js | 32 +- tests/browser/cookies.browser.test.js | 37 ++ tests/browser/xsrf.browser.test.js | 4 +- tests/unit/adapters/fetch.test.js | 98 ++-- tests/unit/adapters/http.test.js | 580 +++++++++++++------ tests/unit/core/AxiosError.test.js | 217 +++++++ tests/unit/helpers/formDataToStream.test.js | 90 +-- tests/unit/helpers/shouldBypassProxy.test.js | 96 ++- tests/unit/prototypePollution.test.js | 360 ++++++++++-- 23 files changed, 1505 insertions(+), 362 deletions(-) diff --git a/README.md b/README.md index a1d15c0d..8e330484 100644 --- a/README.md +++ b/README.md @@ -752,6 +752,11 @@ These are the available config options for making requests. Only the `url` is re firstName: 'Fred' }, + // `formDataHeaderPolicy` controls how node.js FormData#getHeaders() is copied. + // 'legacy' (default) copies all returned headers for v1 compatibility. + // 'content-only' copies only Content-Type and Content-Length. + formDataHeaderPolicy: 'legacy', + // syntax alternative to send data into the body // method post // only the value is sent, not the key @@ -846,6 +851,10 @@ These are the available config options for making requests. Only the `url` is re // `maxBodyLength` (Node only option) defines the max size of the http request content in bytes allowed maxBodyLength: 2000, + // `redact` masks matching config keys when AxiosError#toJSON() is called. + // Matching is case-insensitive and recursive. It does not change the request. + redact: ['authorization', 'password'], + // `validateStatus` defines whether to resolve or reject the promise for a given // HTTP response status code. If `validateStatus` returns `true` (or is set to `null` // or `undefined`), the promise will be resolved; otherwise, the promise will be @@ -1360,6 +1369,17 @@ axios.get('/user/12345').catch(function (error) { }); ``` +To avoid logging secrets from `error.config`, pass a `redact` array in the request config. Matching config keys are masked case-insensitively at any depth when `AxiosError#toJSON()` is called. + +```js +axios.get('/user/12345', { + headers: { Authorization: 'Bearer token' }, + redact: ['authorization'] +}).catch(function (error) { + console.log(error.toJSON().config.headers.Authorization); // [REDACTED ****] +}); +``` + ## Handling Timeouts ```js @@ -1601,6 +1621,8 @@ form.append('my_file', fs.createReadStream('/foo/bar.jpg')); axios.post('https://example.com', form); ``` +In node.js, when a `FormData` object provides `getHeaders()`, axios copies all returned headers by default for v1 compatibility. If the `FormData` object is custom or not fully trusted, set `formDataHeaderPolicy: 'content-only'` to copy only `Content-Type` and `Content-Length`, and set any other request headers explicitly with the request `headers` config. + ### 🆕 Automatic serialization to FormData Starting from `v0.27.0`, Axios supports automatic object serialization to a FormData object if the request `Content-Type` diff --git a/docs/pages/advanced/error-handling.md b/docs/pages/advanced/error-handling.md index 7b2f1ef1..9053cfa1 100644 --- a/docs/pages/advanced/error-handling.md +++ b/docs/pages/advanced/error-handling.md @@ -70,3 +70,14 @@ axios.get("/user/12345").catch(function (error) { console.log(error.toJSON()); }); ``` + +To avoid logging secrets from `error.config`, pass a `redact` array in the request config. Matching config keys are masked case-insensitively at any depth when `AxiosError#toJSON()` is called. + +```js +axios.get("/user/12345", { + headers: { Authorization: "Bearer token" }, + redact: ["authorization"] +}).catch(function (error) { + console.log(error.toJSON().config.headers.Authorization); // [REDACTED ****] +}); +``` diff --git a/docs/pages/advanced/request-config.md b/docs/pages/advanced/request-config.md index da04404a..158fdfe0 100644 --- a/docs/pages/advanced/request-config.md +++ b/docs/pages/advanced/request-config.md @@ -108,6 +108,12 @@ The `data` is the data to be sent as the request body. This can be a string, a p - Browser only: FormData, File, Blob - Node only: Stream, Buffer, FormData (form-data package) +For Node.js `FormData` objects that provide a `getHeaders()` method, axios copies all returned headers by default for v1 compatibility. If the `FormData` object is custom or not fully trusted, set `formDataHeaderPolicy: 'content-only'` to copy only `Content-Type` and `Content-Length`, and set any other request headers explicitly via the request `headers` config. + +### `formDataHeaderPolicy` + +Controls how axios copies headers returned by Node.js `FormData#getHeaders()`. The default is `'legacy'`, which copies all returned headers to preserve existing v1 behavior. Set `'content-only'` to copy only `Content-Type` and `Content-Length` from `getHeaders()`. + ### `timeout` The `timeout` is the number of milliseconds before the request times out. If the request takes longer than `timeout`, the request will be aborted. @@ -206,6 +212,22 @@ The `maxContentLength` property defines the maximum number of bytes that the ser The `maxBodyLength` property defines the maximum number of bytes that the server will accept in the request. +### `redact` + +The `redact` property is an optional array of config key names to mask when an `AxiosError` is serialized with `toJSON()`. Matching is case-insensitive and recursive across the serialized request config. Matching values are replaced with `[REDACTED ****]`. + +`redact` only affects error serialization. It does not change request data, headers, or the original config object. + +```js +axios.get('/user/12345', { + headers: { Authorization: 'Bearer token' }, + auth: { username: 'me', password: 'secret' }, + redact: ['authorization', 'password'] +}).catch((error) => { + console.log(error.toJSON().config); +}); +``` + ### `validateStatus` The `validateStatus` function allows you to override the default status code validation. By default, axios will reject the promise if the status code is not in the range of 200-299. You can override this behavior by providing a custom `validateStatus` function. The function should return `true` if the status code is within the range you want to accept. @@ -362,6 +384,7 @@ The `maxRate` property defines the maximum **bandwidth** (in bytes per second) f data: { firstName: "Fred" }, + formDataHeaderPolicy: "legacy", // Syntax alternative to send data into the body method post only the value is sent, not the key data: "Country=Brasil&City=Belo Horizonte", timeout: 1000, @@ -387,6 +410,7 @@ The `maxRate` property defines the maximum **bandwidth** (in bytes per second) f }, maxContentLength: 2000, maxBodyLength: 2000, + redact: ['authorization', 'password'], validateStatus: function (status) { return status >= 200 && status < 300; }, diff --git a/index.d.cts b/index.d.cts index 016f6cfc..918e4821 100644 --- a/index.d.cts +++ b/index.d.cts @@ -552,6 +552,8 @@ declare namespace axios { http2Options?: Record & { sessionTimeout?: number; }; + formDataHeaderPolicy?: 'legacy' | 'content-only'; + redact?: string[]; } // Alias diff --git a/index.d.ts b/index.d.ts index 100da68b..fea8b7f4 100644 --- a/index.d.ts +++ b/index.d.ts @@ -447,6 +447,8 @@ export interface AxiosRequestConfig { http2Options?: Record & { sessionTimeout?: number; }; + formDataHeaderPolicy?: 'legacy' | 'content-only'; + redact?: string[]; } // Alias diff --git a/lib/adapters/adapters.js b/lib/adapters/adapters.js index 5d18745a..68f675f3 100644 --- a/lib/adapters/adapters.js +++ b/lib/adapters/adapters.js @@ -25,11 +25,13 @@ const knownAdapters = { utils.forEach(knownAdapters, (fn, value) => { if (fn) { try { - Object.defineProperty(fn, 'name', { value }); + // Null-proto descriptors so a polluted Object.prototype.get cannot turn + // these data descriptors into accessor descriptors on the way in. + Object.defineProperty(fn, 'name', { __proto__: null, value }); } catch (e) { // eslint-disable-next-line no-empty } - Object.defineProperty(fn, 'adapterName', { value }); + Object.defineProperty(fn, 'adapterName', { __proto__: null, value }); } }); diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 057ae811..252c7d87 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -47,6 +47,20 @@ const isBrotliSupported = utils.isFunction(zlib.createBrotliDecompress); const { http: httpFollow, https: httpsFollow } = followRedirects; const isHttps = /https:?/; +const FORM_DATA_CONTENT_HEADERS = ['content-type', 'content-length']; + +function setFormDataHeaders(headers, formHeaders, policy) { + if (policy !== 'content-only') { + headers.set(formHeaders); + return; + } + + Object.entries(formHeaders).forEach(([key, val]) => { + if (FORM_DATA_CONTENT_HEADERS.includes(key.toLowerCase())) { + headers.set(key, val); + } + }); +} // Symbols used to bind a single 'error' listener to a pooled socket and track // the request currently owning that socket across keep-alive reuse (issue #10780). @@ -232,22 +246,43 @@ function setProxy(options, configProxy, location, isRedirect) { } } if (proxy) { + // Read proxy fields without traversing the prototype chain. URL instances expose + // username/password/hostname/host/port/protocol via getters on URL.prototype (so + // direct reads are shielded), but plain object proxies — and the `auth` field + // (which URL does not expose) — must be guarded so a polluted Object.prototype + // (e.g. Object.prototype.auth = { username, password }) cannot inject + // attacker-controlled credentials into the Proxy-Authorization header or + // redirect proxying to an attacker-controlled host. + const isProxyURL = proxy instanceof URL; + const readProxyField = (key) => + isProxyURL || utils.hasOwnProp(proxy, key) ? proxy[key] : undefined; + + const proxyUsername = readProxyField('username'); + const proxyPassword = readProxyField('password'); + let proxyAuth = utils.hasOwnProp(proxy, 'auth') ? proxy.auth : undefined; + // Basic proxy authorization - if (proxy.username) { - proxy.auth = (proxy.username || '') + ':' + (proxy.password || ''); + if (proxyUsername) { + proxyAuth = (proxyUsername || '') + ':' + (proxyPassword || ''); } - if (proxy.auth) { - // Support proxy auth object form - const validProxyAuth = Boolean(proxy.auth.username || proxy.auth.password); + if (proxyAuth) { + // Support proxy auth object form. Read sub-fields via own-prop checks so a + // plain object inheriting from polluted Object.prototype cannot leak creds. + const authIsObject = typeof proxyAuth === 'object'; + const authUsername = + authIsObject && utils.hasOwnProp(proxyAuth, 'username') ? proxyAuth.username : undefined; + const authPassword = + authIsObject && utils.hasOwnProp(proxyAuth, 'password') ? proxyAuth.password : undefined; + const validProxyAuth = Boolean(authUsername || authPassword); if (validProxyAuth) { - proxy.auth = (proxy.auth.username || '') + ':' + (proxy.auth.password || ''); - } else if (typeof proxy.auth === 'object') { + proxyAuth = (authUsername || '') + ':' + (authPassword || ''); + } else if (authIsObject) { throw new AxiosError('Invalid proxy authorization', AxiosError.ERR_BAD_OPTION, { proxy }); } - const base64 = Buffer.from(proxy.auth, 'utf8').toString('base64'); + const base64 = Buffer.from(proxyAuth, 'utf8').toString('base64'); options.headers['Proxy-Authorization'] = 'Basic ' + base64; } @@ -255,7 +290,7 @@ function setProxy(options, configProxy, location, isRedirect) { // Preserve a user-supplied Host header (case-insensitive) so callers can override // the value forwarded to the proxy; otherwise default to the request URL's host. let hasUserHostHeader = false; - for (const name in options.headers) { + for (const name of Object.keys(options.headers)) { if (name.toLowerCase() === 'host') { hasUserHostHeader = true; break; @@ -264,14 +299,15 @@ function setProxy(options, configProxy, location, isRedirect) { if (!hasUserHostHeader) { options.headers.host = options.hostname + (options.port ? ':' + options.port : ''); } - const proxyHost = proxy.hostname || proxy.host; + const proxyHost = readProxyField('hostname') || readProxyField('host'); options.hostname = proxyHost; // Replace 'host' since options is not a URL object options.host = proxyHost; - options.port = proxy.port; + options.port = readProxyField('port'); options.path = location; - if (proxy.protocol) { - options.protocol = proxy.protocol.includes(':') ? proxy.protocol : `${proxy.protocol}:`; + const proxyProtocol = readProxyField('protocol'); + if (proxyProtocol) { + options.protocol = proxyProtocol.includes(':') ? proxyProtocol : `${proxyProtocol}:`; } } @@ -598,9 +634,12 @@ export default isHttpAdapterSupported && } ); // support for https://www.npmjs.com/package/form-data api - } else if (utils.isFormData(data) && utils.isFunction(data.getHeaders) && - data.getHeaders !== Object.prototype.getHeaders) { - headers.set(data.getHeaders()); + } else if ( + utils.isFormData(data) && + utils.isFunction(data.getHeaders) && + data.getHeaders !== Object.prototype.getHeaders + ) { + setFormDataHeaders(headers, data.getHeaders(), own('formDataHeaderPolicy')); if (!headers.hasContentLength()) { try { @@ -724,7 +763,6 @@ export default isHttpAdapterSupported && // Null-prototype to block prototype pollution gadgets on properties read // directly by Node's http.request (e.g. insecureHTTPParser, lookup). - // See GHSA-q8qp-cvcw-x6jj. const options = Object.assign(Object.create(null), { path, method: method, @@ -743,11 +781,9 @@ export default isHttpAdapterSupported && if (config.socketPath) { if (typeof config.socketPath !== 'string') { - return reject(new AxiosError( - 'socketPath must be a string', - AxiosError.ERR_BAD_OPTION_VALUE, - config - )); + return reject( + new AxiosError('socketPath must be a string', AxiosError.ERR_BAD_OPTION_VALUE, config) + ); } if (config.allowedSocketPaths != null) { @@ -761,11 +797,13 @@ export default isHttpAdapterSupported && ); if (!isAllowed) { - return reject(new AxiosError( - `socketPath "${config.socketPath}" is not permitted by allowedSocketPaths`, - AxiosError.ERR_BAD_OPTION_VALUE, - config - )); + return reject( + new AxiosError( + `socketPath "${config.socketPath}" is not permitted by allowedSocketPaths`, + AxiosError.ERR_BAD_OPTION_VALUE, + config + ) + ); } } @@ -816,7 +854,7 @@ export default isHttpAdapterSupported && // Always set an explicit own value so a polluted // Object.prototype.insecureHTTPParser cannot enable the lenient parser - // through Node's internal options copy (GHSA-q8qp-cvcw-x6jj). + // through Node's internal options copy options.insecureHTTPParser = Boolean(own('insecureHTTPParser')); // Create the request @@ -904,7 +942,7 @@ export default isHttpAdapterSupported && if (responseType === 'stream') { // Enforce maxContentLength on streamed responses; previously this - // was applied only to buffered responses. See GHSA-vf2m-468p-8v99. + // was applied only to buffered responses. if (config.maxContentLength > -1) { const limit = config.maxContentLength; const source = responseStream; @@ -1121,7 +1159,7 @@ export default isHttpAdapterSupported && // Enforce maxBodyLength for streamed uploads on the native http/https // transport (maxRedirects === 0); follow-redirects enforces it on the - // other path. See GHSA-5c9x-8gcm-mpgx. + // other path. let uploadStream = data; if (config.maxBodyLength > -1 && config.maxRedirects === 0) { const limit = config.maxBodyLength; diff --git a/lib/core/AxiosError.js b/lib/core/AxiosError.js index 882bcfb5..d4924854 100644 --- a/lib/core/AxiosError.js +++ b/lib/core/AxiosError.js @@ -1,6 +1,76 @@ 'use strict'; import utils from '../utils.js'; +import AxiosHeaders from './AxiosHeaders.js'; + +const REDACTED = '[REDACTED ****]'; + +function hasOwnOrPrototypeToJSON(source) { + if (utils.hasOwnProp(source, 'toJSON')) { + return true; + } + + let prototype = Object.getPrototypeOf(source); + + while (prototype && prototype !== Object.prototype) { + if (utils.hasOwnProp(prototype, 'toJSON')) { + return true; + } + + prototype = Object.getPrototypeOf(prototype); + } + + return false; +} + +// Build a plain-object snapshot of `config` and replace the value of any key +// (case-insensitive) listed in `redactKeys` with REDACTED. Walks through arrays +// and AxiosHeaders, and short-circuits on circular references. +function redactConfig(config, redactKeys) { + const lowerKeys = new Set(redactKeys.map((k) => String(k).toLowerCase())); + const seen = []; + + const visit = (source) => { + if (source === null || typeof source !== 'object') return source; + if (utils.isBuffer(source)) return source; + if (seen.indexOf(source) !== -1) return undefined; + + if (source instanceof AxiosHeaders) { + source = source.toJSON(); + } + + seen.push(source); + + let result; + if (utils.isArray(source)) { + result = []; + source.forEach((v, i) => { + const reducedValue = visit(v); + if (!utils.isUndefined(reducedValue)) { + result[i] = reducedValue; + } + }); + } else { + if (!utils.isPlainObject(source) && hasOwnOrPrototypeToJSON(source)) { + seen.pop(); + return source; + } + + result = Object.create(null); + for (const [key, value] of Object.entries(source)) { + const reducedValue = lowerKeys.has(key.toLowerCase()) ? REDACTED : visit(value); + if (!utils.isUndefined(reducedValue)) { + result[key] = reducedValue; + } + } + } + + seen.pop(); + return result; + }; + + return visit(config); +} class AxiosError extends Error { static from(error, code, config, request, response, customProps) { @@ -35,6 +105,9 @@ class AxiosError extends Error { // The native Error constructor sets message as non-enumerable, // but axios < v1.13.3 had it as enumerable Object.defineProperty(this, 'message', { + // Null-proto descriptor so a polluted Object.prototype.get cannot turn + // this data descriptor into an accessor descriptor on the way in. + __proto__: null, value: message, enumerable: true, writable: true, @@ -53,6 +126,17 @@ class AxiosError extends Error { } toJSON() { + // Opt-in redaction: when the request config carries a `redact` array, the + // value of any matching key (case-insensitive, at any depth) is replaced + // with REDACTED in the serialized snapshot. Undefined or empty leaves the + // existing serialization behavior unchanged. + const config = this.config; + const redactKeys = config && utils.hasOwnProp(config, 'redact') ? config.redact : undefined; + const serializedConfig = + utils.isArray(redactKeys) && redactKeys.length > 0 + ? redactConfig(config, redactKeys) + : utils.toJSONObject(config); + return { // Standard message: this.message, @@ -66,7 +150,7 @@ class AxiosError extends Error { columnNumber: this.columnNumber, stack: this.stack, // Axios - config: utils.toJSONObject(this.config), + config: serializedConfig, code: this.code, status: this.status, }; diff --git a/lib/core/AxiosHeaders.js b/lib/core/AxiosHeaders.js index f57e98f8..bc411f59 100644 --- a/lib/core/AxiosHeaders.js +++ b/lib/core/AxiosHeaders.js @@ -98,6 +98,9 @@ function buildAccessors(obj, header) { ['get', 'set', 'has'].forEach((methodName) => { Object.defineProperty(obj, methodName + accessorName, { + // Null-proto descriptor so a polluted Object.prototype.get cannot turn + // this data descriptor into an accessor descriptor on the way in. + __proto__: null, value: function (arg1, arg2, arg3) { return this[methodName].call(this, header, arg1, arg2, arg3); }, diff --git a/lib/core/mergeConfig.js b/lib/core/mergeConfig.js index 65e0e394..760f5ad7 100644 --- a/lib/core/mergeConfig.js +++ b/lib/core/mergeConfig.js @@ -19,11 +19,14 @@ export default function mergeConfig(config1, config2) { config2 = config2 || {}; // Use a null-prototype object so that downstream reads such as `config.auth` - // or `config.baseURL` cannot inherit polluted values from Object.prototype - // (see GHSA-q8qp-cvcw-x6jj). `hasOwnProperty` is restored as a non-enumerable - // own slot to preserve ergonomics for user code that relies on it. + // or `config.baseURL` cannot inherit polluted values from Object.prototype. + // `hasOwnProperty` is restored as a non-enumerable own slot to preserve + // ergonomics for user code that relies on it. const config = Object.create(null); Object.defineProperty(config, 'hasOwnProperty', { + // Null-proto descriptor so a polluted Object.prototype.get cannot turn + // this data descriptor into an accessor descriptor on the way in. + __proto__: null, value: Object.prototype.hasOwnProperty, enumerable: false, writable: true, diff --git a/lib/helpers/cookies.js b/lib/helpers/cookies.js index 1553fb29..3f0baf24 100644 --- a/lib/helpers/cookies.js +++ b/lib/helpers/cookies.js @@ -30,8 +30,20 @@ export default platform.hasStandardBrowserEnv read(name) { if (typeof document === 'undefined') return null; - const match = document.cookie.match(new RegExp('(?:^|; )' + name + '=([^;]*)')); - return match ? decodeURIComponent(match[1]) : null; + // Match name=value by splitting on the semicolon separator instead of building a + // RegExp from `name` — interpolating an unescaped string into a RegExp would let + // metacharacters (e.g. `.+?` in an attacker-influenced cookie name) cause ReDoS or + // match the wrong cookie. Browsers may serialize cookie pairs as either ";" or + // "; ", so ignore optional whitespace before each cookie name. + const cookies = document.cookie.split(';'); + for (let i = 0; i < cookies.length; i++) { + const cookie = cookies[i].replace(/^\s+/, ''); + const eq = cookie.indexOf('='); + if (eq !== -1 && cookie.slice(0, eq) === name) { + return decodeURIComponent(cookie.slice(eq + 1)); + } + } + return null; }, remove(name) { diff --git a/lib/helpers/resolveConfig.js b/lib/helpers/resolveConfig.js index 4d931c64..43a49c53 100644 --- a/lib/helpers/resolveConfig.js +++ b/lib/helpers/resolveConfig.js @@ -7,6 +7,21 @@ import mergeConfig from '../core/mergeConfig.js'; import AxiosHeaders from '../core/AxiosHeaders.js'; import buildURL from './buildURL.js'; +const FORM_DATA_CONTENT_HEADERS = ['content-type', 'content-length']; + +function setFormDataHeaders(headers, formHeaders, policy) { + if (policy !== 'content-only') { + headers.set(formHeaders); + return; + } + + Object.entries(formHeaders).forEach(([key, val]) => { + if (FORM_DATA_CONTENT_HEADERS.includes(key.toLowerCase())) { + headers.set(key, val); + } + }); +} + /** * Encode a UTF-8 string to a Latin-1 byte string for use with btoa(). * This is a modern replacement for the deprecated unescape(encodeURIComponent(str)) pattern. @@ -15,16 +30,16 @@ import buildURL from './buildURL.js'; * * @returns {string} UTF-8 bytes as a Latin-1 string */ -const encodeUTF8 = (str) => encodeURIComponent(str).replace( - /%([0-9A-F]{2})/gi, - (_, hex) => String.fromCharCode(parseInt(hex, 16)) -); +const encodeUTF8 = (str) => + encodeURIComponent(str).replace(/%([0-9A-F]{2})/gi, (_, hex) => + String.fromCharCode(parseInt(hex, 16)) + ); export default (config) => { const newConfig = mergeConfig({}, config); // Read only own properties to prevent prototype pollution gadgets - // (e.g. Object.prototype.baseURL = 'https://evil.com'). See GHSA-q8qp-cvcw-x6jj. + // (e.g. Object.prototype.baseURL = 'https://evil.com'). const own = (key) => (utils.hasOwnProp(newConfig, key) ? newConfig[key] : undefined); const data = own('data'); @@ -47,8 +62,10 @@ export default (config) => { // HTTP basic authentication if (auth) { - headers.set('Authorization', 'Basic ' + - btoa((auth.username || '') + ':' + (auth.password ? encodeUTF8(auth.password) : '')) + headers.set( + 'Authorization', + 'Basic ' + + btoa((auth.username || '') + ':' + (auth.password ? encodeUTF8(auth.password) : '')) ); } @@ -57,14 +74,7 @@ export default (config) => { headers.setContentType(undefined); // browser handles it } else if (utils.isFunction(data.getHeaders)) { // Node.js FormData (like form-data package) - const formHeaders = data.getHeaders(); - // Only set safe headers to avoid overwriting security headers - const allowedHeaders = ['content-type', 'content-length']; - Object.entries(formHeaders).forEach(([key, val]) => { - if (allowedHeaders.includes(key.toLowerCase())) { - headers.set(key, val); - } - }); + setFormDataHeaders(headers, data.getHeaders(), own('formDataHeaderPolicy')); } } @@ -79,10 +89,9 @@ export default (config) => { // Strict boolean check — prevents proto-pollution gadgets (e.g. Object.prototype.withXSRFToken = 1) // and misconfigurations (e.g. "false") from short-circuiting the same-origin check and leaking - // the XSRF token cross-origin. See GHSA-xx6v-rp6x-q39c. + // the XSRF token cross-origin. const shouldSendXSRF = - withXSRFToken === true || - (withXSRFToken == null && isURLSameOrigin(newConfig.url)); + withXSRFToken === true || (withXSRFToken == null && isURLSameOrigin(newConfig.url)); if (shouldSendXSRF) { const xsrfValue = xsrfHeaderName && xsrfCookieName && cookies.read(xsrfCookieName); diff --git a/lib/helpers/shouldBypassProxy.js b/lib/helpers/shouldBypassProxy.js index 95a85e1f..7f61a1b1 100644 --- a/lib/helpers/shouldBypassProxy.js +++ b/lib/helpers/shouldBypassProxy.js @@ -87,6 +87,31 @@ const parseNoProxyEntry = (entry) => { return [entryHost, entryPort]; }; +// Convert IPv4-mapped IPv6 (::ffff:0:0/96 prefix) to IPv4 dotted form so both +// sides of a NO_PROXY comparison see the same canonical address. Without this, +// `NO_PROXY=192.168.1.5` would not match a request to `http://[::ffff:192.168.1.5]/` +// (Node's URL parser normalises that to `[::ffff:c0a8:105]`), and vice-versa, +// allowing the proxy-bypass policy to be circumvented by using the alternate +// representation. Returns the input unchanged when not IPv4-mapped. +const IPV4_MAPPED_DOTTED_RE = /^(?:::|(?:0{1,4}:){1,4}:|(?:0{1,4}:){5})ffff:(\d+\.\d+\.\d+\.\d+)$/i; +const IPV4_MAPPED_HEX_RE = /^(?:::|(?:0{1,4}:){1,4}:|(?:0{1,4}:){5})ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/i; + +const unmapIPv4MappedIPv6 = (host) => { + if (typeof host !== 'string' || host.indexOf(':') === -1) return host; + + const dotted = host.match(IPV4_MAPPED_DOTTED_RE); + if (dotted) return dotted[1]; + + const hex = host.match(IPV4_MAPPED_HEX_RE); + if (hex) { + const high = parseInt(hex[1], 16); + const low = parseInt(hex[2], 16); + return `${high >> 8}.${high & 0xff}.${low >> 8}.${low & 0xff}`; + } + + return host; +}; + const normalizeNoProxyHost = (hostname) => { if (!hostname) { return hostname; @@ -96,7 +121,7 @@ const normalizeNoProxyHost = (hostname) => { hostname = hostname.slice(1, -1); } - return hostname.replace(/\.+$/, ''); + return unmapIPv4MappedIPv6(hostname.replace(/\.+$/, '')); }; export default function shouldBypassProxy(location) { diff --git a/lib/helpers/validator.js b/lib/helpers/validator.js index e84947a8..077f34d0 100644 --- a/lib/helpers/validator.js +++ b/lib/helpers/validator.js @@ -87,7 +87,7 @@ function assertOptions(options, schema, allowUnknown) { while (i-- > 0) { const opt = keys[i]; // Use hasOwnProperty so a polluted Object.prototype. cannot supply - // a non-function validator and cause a TypeError. See GHSA-q8qp-cvcw-x6jj. + // a non-function validator and cause a TypeError. const validator = Object.prototype.hasOwnProperty.call(schema, opt) ? schema[opt] : undefined; if (validator) { const value = options[opt]; diff --git a/lib/utils.js b/lib/utils.js index ae2a440b..ab6b11f4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -192,21 +192,21 @@ const isFile = kindOfTest('File'); * also have a `name` and `type` attribute to specify filename and content type * * @see https://github.com/facebook/react-native/blob/26684cf3adf4094eb6c405d345a75bf8c7c0bf88/Libraries/Network/FormData.js#L68-L71 - * + * * @param {*} value The value to test - * + * * @returns {boolean} True if value is a React Native Blob, otherwise false */ const isReactNativeBlob = (value) => { return !!(value && typeof value.uri !== 'undefined'); -} +}; /** * Determine if environment is React Native * ReactNative `FormData` has a non-standard `getParts()` method - * + * * @param {*} formData The formData to test - * + * * @returns {boolean} True if environment is React Native, otherwise false */ const isReactNative = (formData) => formData && typeof formData.getParts !== 'undefined'; @@ -259,14 +259,16 @@ const FormDataCtor = typeof G.FormData !== 'undefined' ? G.FormData : undefined; const isFormData = (thing) => { if (!thing) return false; if (FormDataCtor && thing instanceof FormDataCtor) return true; - // Reject plain objects inheriting directly from Object.prototype so prototype-pollution gadgets can't spoof FormData (GHSA-6chq-wfr3-2hj9). + // Reject plain objects inheriting directly from Object.prototype so prototype-pollution gadgets can't spoof FormData. const proto = getPrototypeOf(thing); if (!proto || proto === Object.prototype) return false; if (!isFunction(thing.append)) return false; const kind = kindOf(thing); - return kind === 'formdata' || + return ( + kind === 'formdata' || // detect form-data instance - (kind === 'object' && isFunction(thing.toString) && thing.toString() === '[object FormData]'); + (kind === 'object' && isFunction(thing.toString) && thing.toString() === '[object FormData]') + ); }; /** @@ -411,8 +413,12 @@ function merge(...objs) { } const targetKey = (caseless && findKey(result, key)) || key; - if (isPlainObject(result[targetKey]) && isPlainObject(val)) { - result[targetKey] = merge(result[targetKey], val); + // Read via own-prop only — a bare `result[targetKey]` walks the prototype + // chain, so a polluted Object.prototype value could surface here and get + // copied into the merged result. + const existing = hasOwnProperty(result, targetKey) ? result[targetKey] : undefined; + if (isPlainObject(existing) && isPlainObject(val)) { + result[targetKey] = merge(existing, val); } else if (isPlainObject(val)) { result[targetKey] = merge({}, val); } else if (isArray(val)) { @@ -445,6 +451,9 @@ const extend = (a, b, thisArg, { allOwnKeys } = {}) => { (val, key) => { if (thisArg && isFunction(val)) { Object.defineProperty(a, key, { + // Null-proto descriptor so a polluted Object.prototype.get cannot + // hijack defineProperty's accessor-vs-data resolution. + __proto__: null, value: bind(val, thisArg), writable: true, enumerable: true, @@ -452,6 +461,7 @@ const extend = (a, b, thisArg, { allOwnKeys } = {}) => { }); } else { Object.defineProperty(a, key, { + __proto__: null, value: val, writable: true, enumerable: true, @@ -490,12 +500,14 @@ const stripBOM = (content) => { const inherits = (constructor, superConstructor, props, descriptors) => { constructor.prototype = Object.create(superConstructor.prototype, descriptors); Object.defineProperty(constructor.prototype, 'constructor', { + __proto__: null, value: constructor, writable: true, enumerable: false, configurable: true, }); Object.defineProperty(constructor, 'super', { + __proto__: null, value: superConstructor.prototype, }); props && Object.assign(constructor.prototype, props); diff --git a/tests/browser/cookies.browser.test.js b/tests/browser/cookies.browser.test.js index 6a2a3e1c..0baa89c5 100644 --- a/tests/browser/cookies.browser.test.js +++ b/tests/browser/cookies.browser.test.js @@ -41,6 +41,27 @@ describe('helpers::cookies (vitest browser)', () => { expect(cookies.read('bar')).toBe('def'); }); + it('reads cookies when the cookie separator has no following space', () => { + const descriptor = Object.getOwnPropertyDescriptor(document, 'cookie'); + + Object.defineProperty(document, 'cookie', { + configurable: true, + get() { + return 'foo=abc;bar=def'; + }, + }); + + try { + expect(cookies.read('bar')).toBe('def'); + } finally { + if (descriptor) { + Object.defineProperty(document, 'cookie', descriptor); + } else { + delete document.cookie; + } + } + }); + it('removes cookies', () => { cookies.write('foo', 'bar'); cookies.remove('foo'); @@ -53,4 +74,20 @@ describe('helpers::cookies (vitest browser)', () => { expect(document.cookie).toBe('foo=bar%20baz%25'); }); + + it('matches cookie names exactly even when the name contains regex metacharacters', () => { + // previously cookies.read built a RegExp by interpolating + // the requested name. Metacharacters could match a different cookie or trigger + // catastrophic backtracking. A name such as "X.Y" must not match a cookie called + // "XAY" set by the same site. + cookies.write('XAY', 'wrong'); + + expect(cookies.read('X.Y')).toBeNull(); + }); + + it('does not return a partial match for a name that is a prefix of another cookie', () => { + cookies.write('xsrf-token-extra', 'wrong'); + + expect(cookies.read('xsrf-token')).toBeNull(); + }); }); diff --git a/tests/browser/xsrf.browser.test.js b/tests/browser/xsrf.browser.test.js index ac2170ff..7c9737fc 100644 --- a/tests/browser/xsrf.browser.test.js +++ b/tests/browser/xsrf.browser.test.js @@ -180,9 +180,9 @@ describe('xsrf (vitest browser)', () => { }); }); - // GHSA-xx6v-rp6x-q39c: non-boolean truthy withXSRFToken must not short-circuit + // Non-boolean truthy withXSRFToken must not short-circuit // the same-origin check and leak the XSRF token cross-origin. - describe('GHSA-xx6v-rp6x-q39c non-boolean withXSRFToken', () => { + describe('non-boolean withXSRFToken', () => { afterEach(() => { delete Object.prototype.withXSRFToken; }); diff --git a/tests/unit/adapters/fetch.test.js b/tests/unit/adapters/fetch.test.js index f7d4664d..5661a072 100644 --- a/tests/unit/adapters/fetch.test.js +++ b/tests/unit/adapters/fetch.test.js @@ -582,35 +582,39 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => // Timing-sensitive: a 50ms abort race observed by a fake fetch can flake // under CI runner load even though the production code is fine. Retry as // a backstop. - it('should surface ETIMEDOUT when fetch rejects with a broken DOMException on abort (Safari)', { retry: 2 }, async () => { - const safariFetch = (url, init) => { - const signal = getFetchSignal(url, init); + it( + 'should surface ETIMEDOUT when fetch rejects with a broken DOMException on abort (Safari)', + { retry: 2 }, + async () => { + const safariFetch = (url, init) => { + const signal = getFetchSignal(url, init); - return new Promise((_resolve, reject) => { - const onAbort = () => { - signal.removeEventListener('abort', onAbort); - reject(createBrokenDOMExceptionLikeError()); - }; + return new Promise((_resolve, reject) => { + const onAbort = () => { + signal.removeEventListener('abort', onAbort); + reject(createBrokenDOMExceptionLikeError()); + }; - if (signal.aborted) return onAbort(); - signal.addEventListener('abort', onAbort); - }); - }; + if (signal.aborted) return onAbort(); + signal.addEventListener('abort', onAbort); + }); + }; - await assert.rejects( - () => - fetchAxios.get('/', { - timeout: 50, - env: { fetch: safariFetch }, - }), - (err) => { - assert.strictEqual(err.name, 'AxiosError'); - assert.strictEqual(err.code, 'ETIMEDOUT'); - assert.match(err.message, /timeout of 50ms exceeded/); - return true; - } - ); - }); + await assert.rejects( + () => + fetchAxios.get('/', { + timeout: 50, + env: { fetch: safariFetch }, + }), + (err) => { + assert.strictEqual(err.name, 'AxiosError'); + assert.strictEqual(err.code, 'ETIMEDOUT'); + assert.match(err.message, /timeout of 50ms exceeded/); + return true; + } + ); + } + ); }); it('should combine baseURL and url', async () => { @@ -629,7 +633,9 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => const server = await startHTTPServer( (req, res) => { let body = ''; - req.on('data', (chunk) => { body += chunk; }); + req.on('data', (chunk) => { + body += chunk; + }); req.on('end', () => { res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify({ method: req.method, url: req.url, body })); @@ -639,10 +645,9 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => ); try { - const { data } = await fetchAxios.query( - `http://localhost:${server.address().port}/search`, - { selector: 'field1' } - ); + const { data } = await fetchAxios.query(`http://localhost:${server.address().port}/search`, { + selector: 'field1', + }); assert.strictEqual(data.method, 'QUERY'); assert.strictEqual(data.url, '/search'); @@ -945,7 +950,7 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => }); }); - describe('size limits (GHSA-777c-7fjr-54vf)', () => { + describe('size limits', () => { it('should reject an outbound body that exceeds maxBodyLength with ERR_BAD_REQUEST', async () => { const server = await startHTTPServer( (req, res) => { @@ -1034,20 +1039,18 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => it('should reject a data: URL whose decoded size exceeds maxContentLength (base64)', async () => { const payload = 'A'.repeat(4096); - const dataUrl = 'data:application/octet-stream;base64,' + Buffer.from(payload).toString('base64'); + const dataUrl = + 'data:application/octet-stream;base64,' + Buffer.from(payload).toString('base64'); // Use a dedicated instance without baseURL — combineURLs would otherwise // prepend baseURL to a data: URL and neutralise the pre-check. const bareAxios = axios.create({ adapter: 'fetch' }); - await assert.rejects( - bareAxios.get(dataUrl, { maxContentLength: 16 }), - (err) => { - assert.strictEqual(err.code, 'ERR_BAD_RESPONSE'); - assert.match(err.message, /maxContentLength size of 16 exceeded/); - return true; - } - ); + await assert.rejects(bareAxios.get(dataUrl, { maxContentLength: 16 }), (err) => { + assert.strictEqual(err.code, 'ERR_BAD_RESPONSE'); + assert.match(err.message, /maxContentLength size of 16 exceeded/); + return true; + }); }); it('should reject a data: URL whose body size exceeds maxContentLength (non-base64)', async () => { @@ -1055,14 +1058,11 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => const bareAxios = axios.create({ adapter: 'fetch' }); - await assert.rejects( - bareAxios.get(dataUrl, { maxContentLength: 16 }), - (err) => { - assert.strictEqual(err.code, 'ERR_BAD_RESPONSE'); - assert.match(err.message, /maxContentLength size of 16 exceeded/); - return true; - } - ); + await assert.rejects(bareAxios.get(dataUrl, { maxContentLength: 16 }), (err) => { + assert.strictEqual(err.code, 'ERR_BAD_RESPONSE'); + assert.match(err.message, /maxContentLength size of 16 exceeded/); + return true; + }); }); it('should allow a response at or below maxContentLength', async () => { diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js index 1041f452..1b1ef534 100644 --- a/tests/unit/adapters/http.test.js +++ b/tests/unit/adapters/http.test.js @@ -1012,9 +1012,7 @@ describe('supports http with nodejs', () => { ); try { - const response = await axios.get( - `http://user%:foo%zz@localhost:${server.address().port}/` - ); + const response = await axios.get(`http://user%:foo%zz@localhost:${server.address().port}/`); const base64 = Buffer.from('user%:foo%zz', 'utf8').toString('base64'); assert.strictEqual(response.data, `Basic ${base64}`); } finally { @@ -1184,7 +1182,7 @@ describe('supports http with nodejs', () => { } }); - it('should enforce maxContentLength for streamed responses (GHSA-vf2m-468p-8v99)', async () => { + it('should enforce maxContentLength for streamed responses', async () => { const size = 2 * 1024 * 1024; const body = Buffer.alloc(size, 0x63); const server = await startHTTPServer( @@ -1203,17 +1201,16 @@ describe('supports http with nodejs', () => { let bytesRead = 0; const err = await new Promise((resolve) => { - response.data.on('data', (chunk) => { bytesRead += chunk.length; }); + response.data.on('data', (chunk) => { + bytesRead += chunk.length; + }); response.data.on('error', resolve); response.data.on('end', () => resolve(null)); }); assert.ok(err, 'stream should emit an error'); assert.strictEqual(err.message, 'maxContentLength size of 1024 exceeded'); - assert.ok( - bytesRead <= 1024 * 64, - `stream should not deliver full payload; got ${bytesRead}` - ); + assert.ok(bytesRead <= 1024 * 64, `stream should not deliver full payload; got ${bytesRead}`); } finally { await stopHTTPServer(server); } @@ -1248,7 +1245,7 @@ describe('supports http with nodejs', () => { } }); - it('should enforce maxBodyLength for streamed uploads with maxRedirects: 0 (GHSA-5c9x-8gcm-mpgx)', async () => { + it('should enforce maxBodyLength for streamed uploads with maxRedirects: 0', async () => { let bytesReceived = 0; const server = await startHTTPServer( (req, res) => { @@ -1308,15 +1305,11 @@ describe('supports http with nodejs', () => { const payload = Buffer.alloc(512, 0x62); const source = stream.Readable.from([payload]); - const response = await axios.post( - `http://localhost:${server.address().port}/`, - source, - { - maxBodyLength: 1024, - maxRedirects: 0, - headers: { 'Content-Type': 'application/octet-stream' }, - } - ); + const response = await axios.post(`http://localhost:${server.address().port}/`, source, { + maxBodyLength: 1024, + maxRedirects: 0, + headers: { 'Content-Type': 'application/octet-stream' }, + }); assert.strictEqual(response.data.received, payload.length); } finally { @@ -2559,9 +2552,28 @@ describe('supports http with nodejs', () => { assert.strictEqual(options.headers.Host, 'example.com'); assert.strictEqual(options.headers.host, undefined); }); + + it('ignores polluted prototype Host fields when detecting user-supplied headers', () => { + Object.prototype.host = 'polluted.example.com'; + + const options = { + headers: {}, + beforeRedirects: {}, + hostname: '127.0.0.1', + port: 4000, + }; + + try { + __setProxy(options, proxyConfig, 'http://127.0.0.1:4000/'); + + assert.strictEqual(options.headers.host, '127.0.0.1:4000'); + } finally { + delete Object.prototype.host; + } + }); }); - describe('Proxy-Authorization header leak on redirect (GHSA-j5f8-grm9-p9fc)', () => { + describe('Proxy-Authorization header leak on redirect', () => { it('clears a stale Proxy-Authorization header when redirected request resolves to no proxy (configProxy=false)', () => { const options = { headers: {}, @@ -2571,7 +2583,11 @@ describe('supports http with nodejs', () => { port: 80, }; - __setProxy(options, { host: '127.0.0.1', port: 8030, auth: { username: 'user', password: 'pass' } }, 'http://initial.example.com/start'); + __setProxy( + options, + { host: '127.0.0.1', port: 8030, auth: { username: 'user', password: 'pass' } }, + 'http://initial.example.com/start' + ); assert.strictEqual( options.headers['Proxy-Authorization'], 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64'), @@ -2637,9 +2653,12 @@ describe('supports http with nodejs', () => { 'stale Proxy-Authorization must be stripped when redirect target is covered by NO_PROXY' ); } finally { - if (originalHttpProxy === undefined) delete process.env.http_proxy; else process.env.http_proxy = originalHttpProxy; - if (originalHttpsProxy === undefined) delete process.env.https_proxy; else process.env.https_proxy = originalHttpsProxy; - if (originalNoProxy === undefined) delete process.env.no_proxy; else process.env.no_proxy = originalNoProxy; + if (originalHttpProxy === undefined) delete process.env.http_proxy; + else process.env.http_proxy = originalHttpProxy; + if (originalHttpsProxy === undefined) delete process.env.https_proxy; + else process.env.https_proxy = originalHttpsProxy; + if (originalNoProxy === undefined) delete process.env.no_proxy; + else process.env.no_proxy = originalNoProxy; } }); @@ -2652,8 +2671,15 @@ describe('supports http with nodejs', () => { port: 80, }; - __setProxy(options, { host: '127.0.0.1', port: 8030, auth: { username: 'user', password: 'pass' } }, 'http://initial.example.com/start'); - assert.ok(options.headers['Proxy-Authorization'], 'precondition: initial proxy auth header set'); + __setProxy( + options, + { host: '127.0.0.1', port: 8030, auth: { username: 'user', password: 'pass' } }, + 'http://initial.example.com/start' + ); + assert.ok( + options.headers['Proxy-Authorization'], + 'precondition: initial proxy auth header set' + ); const redirectOptions = { headers: { ...options.headers }, @@ -2662,7 +2688,12 @@ describe('supports http with nodejs', () => { host: 'second.example.com', port: 80, }; - __setProxy(redirectOptions, { host: '127.0.0.2', port: 8031 }, 'http://second.example.com/final', true); + __setProxy( + redirectOptions, + { host: '127.0.0.2', port: 8031 }, + 'http://second.example.com/final', + true + ); assert.strictEqual( redirectOptions.headers['Proxy-Authorization'], @@ -2673,7 +2704,9 @@ describe('supports http with nodejs', () => { it('strips stale Proxy-Authorization when the beforeRedirects.proxy hook is invoked with configProxy=false', () => { const options = { - headers: { 'Proxy-Authorization': 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64') }, + headers: { + 'Proxy-Authorization': 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64'), + }, beforeRedirects: {}, hostname: 'initial.example.com', host: 'initial.example.com', @@ -2681,10 +2714,16 @@ describe('supports http with nodejs', () => { }; __setProxy(options, false, 'http://initial.example.com/start'); - assert.strictEqual(typeof options.beforeRedirects.proxy, 'function', 'initial setProxy must install redirect hook'); + assert.strictEqual( + typeof options.beforeRedirects.proxy, + 'function', + 'initial setProxy must install redirect hook' + ); const redirectOptions = { - headers: { 'Proxy-Authorization': 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64') }, + headers: { + 'Proxy-Authorization': 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64'), + }, beforeRedirects: {}, hostname: 'attacker.example.com', host: 'attacker.example.com', @@ -2722,7 +2761,12 @@ describe('supports http with nodejs', () => { it('strips stale Proxy-Authorization regardless of header key casing', () => { const staleValue = 'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64'); - const casings = ['proxy-authorization', 'PROXY-AUTHORIZATION', 'Proxy-authorization', 'pRoXy-AuThOrIzAtIoN']; + const casings = [ + 'proxy-authorization', + 'PROXY-AUTHORIZATION', + 'Proxy-authorization', + 'pRoXy-AuThOrIzAtIoN', + ]; for (const casing of casings) { const redirectOptions = { @@ -2745,6 +2789,77 @@ describe('supports http with nodejs', () => { ); } }); + + // End-to-end exercise of the redirect leak. An + // authenticated env-supplied proxy sees the initial request, 302s the + // client to a target that NO_PROXY excludes, and the redirected request + // must not carry the stale Proxy-Authorization to the direct target. + it('does not forward Proxy-Authorization to a redirect target that resolves to no-proxy', async () => { + const startServer = (handler) => + new Promise((resolve) => { + const s = http.createServer(handler); + s.listen(0, '127.0.0.1', () => resolve(s)); + }); + const stop = (s) => new Promise((r) => s.close(r)); + + let attackerPort; + const proxySaw = []; + const attackerSaw = []; + + // The proxy receives the absolute-form URL (`GET http://target/path`) on + // the initial request, then forwards to the destination. We short-circuit + // by responding directly with the redirect. + const corpProxy = await startServer((req, res) => { + proxySaw.push({ url: req.url, proxyAuth: req.headers['proxy-authorization'] }); + res.writeHead(302, { Location: `http://127.0.0.1:${attackerPort}/final` }); + res.end(); + }); + + const attacker = await startServer((req, res) => { + attackerSaw.push({ + url: req.url, + proxyAuth: req.headers['proxy-authorization'], + authorization: req.headers.authorization, + }); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"final":true}'); + }); + attackerPort = attacker.address().port; + + const corpProxyPort = corpProxy.address().port; + const originalHttpProxy = process.env.http_proxy; + const originalNoProxy = process.env.no_proxy; + process.env.http_proxy = `http://user:pass@127.0.0.1:${corpProxyPort}`; + // NO_PROXY entry covers only the attacker target (port-specific), so the + // initial request still uses the proxy but the redirect resolves direct. + process.env.no_proxy = `127.0.0.1:${attackerPort}`; + + try { + await axios.get('http://example.com/start'); + + assert.ok( + proxySaw.some((h) => h.proxyAuth), + 'precondition: corp proxy must see Proxy-Authorization on the initial request' + ); + assert.strictEqual( + attackerSaw.length, + 1, + 'attacker target must receive exactly the redirected request' + ); + assert.strictEqual( + attackerSaw[0].proxyAuth, + undefined, + 'stale Proxy-Authorization must not leak to the redirect target' + ); + } finally { + if (originalHttpProxy === undefined) delete process.env.http_proxy; + else process.env.http_proxy = originalHttpProxy; + if (originalNoProxy === undefined) delete process.env.no_proxy; + else process.env.no_proxy = originalNoProxy; + await stop(corpProxy); + await stop(attacker); + } + }, 10000); }); it('should support cancel', async () => { @@ -3105,7 +3220,7 @@ describe('supports http with nodejs', () => { }); }); - describe('prototype pollution (GHSA-6chq-wfr3-2hj9)', () => { + describe('prototype pollution', () => { const pollutedKeys = ['getHeaders', 'append', 'pipe', 'on', 'once']; const toStringTagSym = Symbol.toStringTag; @@ -3114,11 +3229,18 @@ describe('supports http with nodejs', () => { Object.prototype.append = () => {}; Object.prototype.getHeaders = () => ({ 'x-injected': 'attacker', - 'authorization': 'Bearer ATTACKER_TOKEN', + authorization: 'Bearer ATTACKER_TOKEN', }); - Object.prototype.pipe = function (d) { if (d && d.end) d.end(); return d; }; - Object.prototype.on = function () { return this; }; - Object.prototype.once = function () { return this; }; + Object.prototype.pipe = function (d) { + if (d && d.end) d.end(); + return d; + }; + Object.prototype.on = function () { + return this; + }; + Object.prototype.once = function () { + return this; + }; } function cleanup() { @@ -3161,7 +3283,7 @@ describe('supports http with nodejs', () => { 'http://stub.invalid/', { userId: 42 }, { - headers: { 'Authorization': 'Bearer VALID_USER_TOKEN' }, + headers: { Authorization: 'Bearer VALID_USER_TOKEN' }, transport: stubTransport, maxRedirects: 0, } @@ -3176,6 +3298,97 @@ describe('supports http with nodejs', () => { assert.notStrictEqual(capturedHeaders['authorization'], 'Bearer ATTACKER_TOKEN'); }); }); + + describe('formDataHeaderPolicy', () => { + function createStubTransport(captureHeaders) { + return { + request(options, handleResponse) { + captureHeaders({ ...options.headers }); + const req = new EventEmitter(); + req.write = () => true; + req.setTimeout = () => {}; + req.destroy = () => {}; + req.end = () => { + const res = new stream.Readable({ read() {} }); + res.statusCode = 200; + res.statusMessage = 'OK'; + res.headers = {}; + res.rawHeaders = []; + res.req = req; + process.nextTick(() => { + handleResponse(res); + res.push(null); + }); + }; + return req; + }, + }; + } + + class CustomFormData extends stream.Readable { + _read() { + this.push(null); + } + append() {} + getHeaders() { + return { + 'content-type': 'multipart/form-data; boundary=----fake', + 'x-injected': 'custom', + 'x-forwarded-for': '10.0.0.1', + authorization: 'Bearer CUSTOM_TOKEN', + host: 'custom.example.com', + }; + } + get [Symbol.toStringTag]() { + return 'FormData'; + } + } + + it('preserves legacy getHeaders() propagation by default', async () => { + let capturedHeaders; + + await axios.post('http://stub.invalid/', new CustomFormData(), { + transport: createStubTransport((headers) => { + capturedHeaders = headers; + }), + maxRedirects: 0, + }); + + assert.ok(capturedHeaders, 'transport was not invoked'); + const ct = capturedHeaders['Content-Type'] || capturedHeaders['content-type']; + assert.match(ct, /multipart\/form-data/); + assert.strictEqual(capturedHeaders['x-injected'], 'custom'); + assert.strictEqual(capturedHeaders['x-forwarded-for'], '10.0.0.1'); + assert.strictEqual( + capturedHeaders.Authorization || capturedHeaders.authorization, + 'Bearer CUSTOM_TOKEN' + ); + assert.strictEqual(capturedHeaders.Host || capturedHeaders.host, 'custom.example.com'); + }); + + it('only copies content headers when formDataHeaderPolicy is content-only', async () => { + let capturedHeaders; + + await axios.post('http://stub.invalid/', new CustomFormData(), { + transport: createStubTransport((headers) => { + capturedHeaders = headers; + }), + maxRedirects: 0, + formDataHeaderPolicy: 'content-only', + }); + + assert.ok(capturedHeaders, 'transport was not invoked'); + const ct = capturedHeaders['Content-Type'] || capturedHeaders['content-type']; + assert.match(ct, /multipart\/form-data/); + assert.strictEqual(capturedHeaders['x-injected'], undefined); + assert.strictEqual(capturedHeaders['x-forwarded-for'], undefined); + assert.strictEqual( + capturedHeaders.Authorization || capturedHeaders.authorization, + undefined + ); + assert.strictEqual(capturedHeaders.Host || capturedHeaders.host, undefined); + }); + }); }); describe('toFormData helper', () => { @@ -4348,126 +4561,137 @@ describe('supports http with nodejs', () => { } }); - it('should use different sessions for requests with different http2Options set', { retry: 2 }, async () => { - const server = await startHTTPServer( - (req, res) => { - setTimeout(() => { - res.end('OK'); - }, 1000); - }, - { - useHTTP2: true, - } - ); - - try { - const localServerURL = `https://localhost:${server.address().port}`; - const http2Axios = createHttp2Axios(localServerURL); - - const [response1, response2] = await Promise.all([ - http2Axios.get(localServerURL, { - http2Options: { - sessionTimeout: 2000, - }, - }), - http2Axios.get(localServerURL, { - http2Options: { - sessionTimeout: 4000, - }, - }), - ]); - - assert.notStrictEqual(response1.request.session, response2.request.session); - assert.deepStrictEqual([response1.data, response2.data], ['OK', 'OK']); - } finally { - await stopHTTPServer(server); - } - }); - - it('should use the same session for request with the same resolved http2Options set', { retry: 2 }, async () => { - const server = await startHTTPServer( - (req, res) => { - setTimeout(() => res.end('OK'), 1000); - }, - { - useHTTP2: true, - } - ); - - try { - const localServerURL = `https://localhost:${server.address().port}`; - const http2Axios = createHttp2Axios(localServerURL); - - const responses = await Promise.all([ - http2Axios.get(localServerURL, { - responseType: 'stream', - }), - http2Axios.get(localServerURL, { - responseType: 'stream', - http2Options: undefined, - }), - http2Axios.get(localServerURL, { - responseType: 'stream', - http2Options: {}, - }), - ]); - - assert.strictEqual(responses[1].data.session, responses[0].data.session); - assert.strictEqual(responses[2].data.session, responses[0].data.session); - - assert.deepStrictEqual(await Promise.all(responses.map(({ data }) => getStream(data))), [ - 'OK', - 'OK', - 'OK', - ]); - } finally { - await stopHTTPServer(server); - } - }); - - it('should use different sessions after previous session timeout', { retry: 2, timeout: 15000 }, async () => { - const server = await startHTTPServer( - (req, res) => { - setTimeout(() => res.end('OK'), 100); - }, - { - useHTTP2: true, - } - ); - - try { - const localServerURL = `https://localhost:${server.address().port}`; - const http2Axios = createHttp2Axios(localServerURL); - - const response1 = await http2Axios.get(localServerURL, { - responseType: 'stream', - http2Options: { - sessionTimeout: 1000, + it( + 'should use different sessions for requests with different http2Options set', + { retry: 2 }, + async () => { + const server = await startHTTPServer( + (req, res) => { + setTimeout(() => { + res.end('OK'); + }, 1000); }, - }); + { + useHTTP2: true, + } + ); - const session1 = response1.data.session; - const data1 = await getStream(response1.data); + try { + const localServerURL = `https://localhost:${server.address().port}`; + const http2Axios = createHttp2Axios(localServerURL); - await setTimeoutAsync(5000); + const [response1, response2] = await Promise.all([ + http2Axios.get(localServerURL, { + http2Options: { + sessionTimeout: 2000, + }, + }), + http2Axios.get(localServerURL, { + http2Options: { + sessionTimeout: 4000, + }, + }), + ]); - const response2 = await http2Axios.get(localServerURL, { - responseType: 'stream', - http2Options: { - sessionTimeout: 1000, - }, - }); - - const session2 = response2.data.session; - const data2 = await getStream(response2.data); - - assert.notStrictEqual(session1, session2); - assert.strictEqual(data1, 'OK'); - assert.strictEqual(data2, 'OK'); - } finally { - await stopHTTPServer(server); + assert.notStrictEqual(response1.request.session, response2.request.session); + assert.deepStrictEqual([response1.data, response2.data], ['OK', 'OK']); + } finally { + await stopHTTPServer(server); + } } - }); + ); + + it( + 'should use the same session for request with the same resolved http2Options set', + { retry: 2 }, + async () => { + const server = await startHTTPServer( + (req, res) => { + setTimeout(() => res.end('OK'), 1000); + }, + { + useHTTP2: true, + } + ); + + try { + const localServerURL = `https://localhost:${server.address().port}`; + const http2Axios = createHttp2Axios(localServerURL); + + const responses = await Promise.all([ + http2Axios.get(localServerURL, { + responseType: 'stream', + }), + http2Axios.get(localServerURL, { + responseType: 'stream', + http2Options: undefined, + }), + http2Axios.get(localServerURL, { + responseType: 'stream', + http2Options: {}, + }), + ]); + + assert.strictEqual(responses[1].data.session, responses[0].data.session); + assert.strictEqual(responses[2].data.session, responses[0].data.session); + + assert.deepStrictEqual( + await Promise.all(responses.map(({ data }) => getStream(data))), + ['OK', 'OK', 'OK'] + ); + } finally { + await stopHTTPServer(server); + } + } + ); + + it( + 'should use different sessions after previous session timeout', + { retry: 2, timeout: 15000 }, + async () => { + const server = await startHTTPServer( + (req, res) => { + setTimeout(() => res.end('OK'), 100); + }, + { + useHTTP2: true, + } + ); + + try { + const localServerURL = `https://localhost:${server.address().port}`; + const http2Axios = createHttp2Axios(localServerURL); + + const response1 = await http2Axios.get(localServerURL, { + responseType: 'stream', + http2Options: { + sessionTimeout: 1000, + }, + }); + + const session1 = response1.data.session; + const data1 = await getStream(response1.data); + + await setTimeoutAsync(5000); + + const response2 = await http2Axios.get(localServerURL, { + responseType: 'stream', + http2Options: { + sessionTimeout: 1000, + }, + }); + + const session2 = response2.data.session; + const data2 = await getStream(response2.data); + + assert.notStrictEqual(session1, session2); + assert.strictEqual(data1, 'OK'); + assert.strictEqual(data2, 'OK'); + } finally { + await stopHTTPServer(server); + } + } + ); }); }); @@ -4922,11 +5146,18 @@ describe('supports http with nodejs', () => { await setTimeoutAsync(0); const firstReq = createdReqs[0]; - assert.ok(firstReq && firstReq.destroyed === false, 'first request must not have been destroyed by a socket error'); + assert.ok( + firstReq && firstReq.destroyed === false, + 'first request must not have been destroyed by a socket error' + ); // Stray socket error after first req has closed: must not destroy firstReq. socket.emit('error', new Error('stray error after close')); - assert.strictEqual(firstReq.destroyed, false, 'socket error after close must not destroy the old request'); + assert.strictEqual( + firstReq.destroyed, + false, + 'socket error after close must not destroy the old request' + ); // Second request claims the socket, then its socket errors. It should reject. const err = await axios @@ -4937,7 +5168,11 @@ describe('supports http with nodejs', () => { assert.strictEqual(err.code, 'EPIPE'); const secondReq = createdReqs[1]; - assert.strictEqual(secondReq.destroyed, true, 'second request should be destroyed by its own active socket error'); + assert.strictEqual( + secondReq.destroyed, + true, + 'second request should be destroyed by its own active socket error' + ); }); }); @@ -5077,13 +5312,13 @@ describe('supports http with nodejs', () => { }, 30000); }); - describe('socketPath security (GHSA-j96w-fp6f-pq6v)', () => { + describe('socketPath security', () => { function makeSocketPath() { const pipe = `axios-socketpath-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`; - return os.platform() === 'win32' ? - `\\\\.\\pipe\\${pipe}` : - path.join(os.tmpdir(), `${pipe}.sock`); + return os.platform() === 'win32' + ? `\\\\.\\pipe\\${pipe}` + : path.join(os.tmpdir(), `${pipe}.sock`); } function startUnixServer(socketPath) { @@ -5092,7 +5327,11 @@ describe('supports http with nodejs', () => { res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ ok: true, url: req.url })); }); - try { fs.unlinkSync(socketPath); } catch (_) { /* noop */ } + try { + fs.unlinkSync(socketPath); + } catch (_) { + /* noop */ + } server.once('error', rejectStart); server.listen(socketPath, () => resolveStart(server)); }); @@ -5101,7 +5340,11 @@ describe('supports http with nodejs', () => { function stopUnixServer(server, socketPath) { return new Promise((done) => { server.close(() => { - try { fs.unlinkSync(socketPath); } catch (_) { /* noop */ } + try { + fs.unlinkSync(socketPath); + } catch (_) { + /* noop */ + } done(); }); }); @@ -5193,15 +5436,12 @@ describe('supports http with nodejs', () => { }); it('rejects non-string socketPath', async () => { - await assert.rejects( - axios.get('http://localhost/echo', { socketPath: 12345 }), - (err) => { - assert.ok(err instanceof AxiosError); - assert.strictEqual(err.code, AxiosError.ERR_BAD_OPTION_VALUE); - assert.match(err.message, /socketPath must be a string/); - return true; - } - ); + await assert.rejects(axios.get('http://localhost/echo', { socketPath: 12345 }), (err) => { + assert.ok(err instanceof AxiosError); + assert.strictEqual(err.code, AxiosError.ERR_BAD_OPTION_VALUE); + assert.match(err.message, /socketPath must be a string/); + return true; + }); }); it('empty allowedSocketPaths array blocks all socketPath values', async () => { diff --git a/tests/unit/core/AxiosError.test.js b/tests/unit/core/AxiosError.test.js index c954086e..63de026d 100644 --- a/tests/unit/core/AxiosError.test.js +++ b/tests/unit/core/AxiosError.test.js @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { isNativeError } from 'node:util/types'; import AxiosError from '../../../lib/core/AxiosError.js'; +import AxiosHeaders from '../../../lib/core/AxiosHeaders.js'; describe('core::AxiosError', () => { it('creates an error with message, config, code, request, response, stack and isAxiosError', () => { @@ -143,4 +144,220 @@ describe('core::AxiosError', () => { expect({ ...error }.message).toBe('Test error message'); expect(Object.getOwnPropertyDescriptor(error, 'message')?.enumerable).toBe(true); }); + + // Opt-in redaction: when `config.redact` is an array of key names, every + // matching key (case-insensitive, at any depth) has its value replaced with + // the redaction marker in the toJSON snapshot. Undefined leaves the legacy + // serialization untouched so existing consumers see no behavior change. + describe('toJSON redaction via config.redact', () => { + it('leaves config untouched when redact is undefined', () => { + const config = { + url: '/api', + auth: { username: 'alice', password: 'secret' }, + }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + + expect(json.config.auth.username).toBe('alice'); + expect(json.config.auth.password).toBe('secret'); + }); + + it('ignores inherited redact accessors', () => { + const prototype = {}; + Object.defineProperty(prototype, 'redact', { + get() { + throw new Error('inherited redact getter should not run'); + }, + }); + + const config = Object.create(prototype); + config.auth = { username: 'alice', password: 'secret' }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + + expect(json.config.auth.username).toBe('alice'); + expect(json.config.auth.password).toBe('secret'); + }); + + it('leaves config untouched when redact is an empty array', () => { + const config = { + auth: { username: 'alice', password: 'secret' }, + redact: [], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + expect(error.toJSON().config.auth.password).toBe('secret'); + }); + + it('replaces top-level matching keys with the redaction marker', () => { + const config = { + url: '/api', + auth: { username: 'alice', password: 'secret' }, + redact: ['auth'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + + expect(json.config.url).toBe('/api'); + expect(json.config.auth).toBe('[REDACTED ****]'); + }); + + it('replaces matching keys at any nesting depth', () => { + const config = { + auth: { username: 'alice', password: 'secret' }, + proxy: { auth: { username: 'pu', password: 'pp' } }, + redact: ['password'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + + expect(json.config.auth.username).toBe('alice'); + expect(json.config.auth.password).toBe('[REDACTED ****]'); + expect(json.config.proxy.auth.password).toBe('[REDACTED ****]'); + expect(json.config.proxy.auth.username).toBe('pu'); + }); + + it('matches case-insensitively', () => { + const config = { + headers: { Authorization: 'Bearer abc' }, + redact: ['authorization'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + expect(error.toJSON().config.headers.Authorization).toBe('[REDACTED ****]'); + }); + + it('redacts headers stored in an AxiosHeaders instance', () => { + const headers = new AxiosHeaders(); + headers.set('Authorization', 'Bearer abc'); + headers.set('X-Trace', 'trace-id'); + + const config = { headers, redact: ['Authorization'] }; + const error = new AxiosError('Boom', 'ECODE', config); + + const serialized = error.toJSON().config.headers; + expect(serialized.Authorization).toBe('[REDACTED ****]'); + expect(serialized['X-Trace']).toBe('trace-id'); + }); + + it('redacts inside arrays of objects', () => { + const config = { + items: [{ token: 't1' }, { token: 't2', name: 'keep' }], + redact: ['token'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + expect(json.config.items[0].token).toBe('[REDACTED ****]'); + expect(json.config.items[1].token).toBe('[REDACTED ****]'); + expect(json.config.items[1].name).toBe('keep'); + }); + + it('does not crash on circular config references', () => { + const config = { auth: { password: 'secret' }, redact: ['password'] }; + config.self = config; + + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + expect(json.config.auth.password).toBe('[REDACTED ****]'); + expect(Object.prototype.hasOwnProperty.call(json.config, 'self')).toBe(false); + }); + + it('preserves legacy toJSONObject handling for values with toJSON', () => { + const issuedAt = new Date('2026-01-01T00:00:00.000Z'); + const endpoint = new URL('https://example.com/users'); + const config = { + issuedAt, + endpoint, + auth: { password: 'secret' }, + redact: ['password'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + const json = error.toJSON(); + + expect(json.config.issuedAt).toBe(issuedAt); + expect(json.config.endpoint).toBe(endpoint); + expect(json.config.auth.password).toBe('[REDACTED ****]'); + }); + + it('does not let a polluted Object.prototype.toJSON bypass redaction', () => { + class Credentials { + constructor() { + this.password = 'secret'; + } + } + + Object.prototype.toJSON = function () { + return this; + }; + + const config = { + auth: { password: 'secret' }, + credentials: new Credentials(), + items: [{ token: 't1' }], + redact: ['password', 'token'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + try { + const json = error.toJSON(); + + expect(json.config.auth.password).toBe('[REDACTED ****]'); + expect(json.config.credentials.password).toBe('[REDACTED ****]'); + expect(json.config.items[0].token).toBe('[REDACTED ****]'); + } finally { + delete Object.prototype.toJSON; + } + }); + + it('copies __proto__ as data without changing the redaction output prototype', () => { + const config = { redact: ['password'] }; + Object.defineProperty(config, '__proto__', { + value: { password: 'secret' }, + enumerable: true, + configurable: true, + }); + + const error = new AxiosError('Boom', 'ECODE', config); + const json = error.toJSON(); + + expect(Object.getPrototypeOf(json.config)).toBe(null); + expect(Object.prototype.hasOwnProperty.call(json.config, '__proto__')).toBe(true); + expect(json.config.__proto__.password).toBe('[REDACTED ****]'); + }); + + it('does not mutate the original config or AxiosHeaders', () => { + const headers = new AxiosHeaders(); + headers.set('Authorization', 'Bearer abc'); + + const config = { + auth: { username: 'alice', password: 'secret' }, + headers, + redact: ['password', 'Authorization'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + error.toJSON(); + + expect(config.auth.password).toBe('secret'); + expect(headers.get('Authorization')).toBe('Bearer abc'); + }); + + it('keeps the redact array itself visible in the snapshot', () => { + const config = { + auth: { password: 'secret' }, + redact: ['password'], + }; + const error = new AxiosError('Boom', 'ECODE', config); + + // Useful for debugging — operators can see what was being redacted. + expect(error.toJSON().config.redact).toEqual(['password']); + }); + }); }); diff --git a/tests/unit/helpers/formDataToStream.test.js b/tests/unit/helpers/formDataToStream.test.js index 68662e2b..958431ca 100644 --- a/tests/unit/helpers/formDataToStream.test.js +++ b/tests/unit/helpers/formDataToStream.test.js @@ -34,14 +34,17 @@ const collect = async (stream) => { return Buffer.concat(chunks).toString('utf8'); }; -describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { +describe('formDataToStream', () => { it('should strip CRLF sequences from blob.type to prevent multipart header injection', async () => { const fd = new SpecFormData(); - fd.append('photo', makeBlobLike({ - type: 'image/jpeg\r\nX-Injected-Header: PWNED\r\nX-Evil: bad', - name: 'photo.jpg', - payload: Buffer.from('PAYLOAD'), - })); + fd.append( + 'photo', + makeBlobLike({ + type: 'image/jpeg\r\nX-Injected-Header: PWNED\r\nX-Evil: bad', + name: 'photo.jpg', + payload: Buffer.from('PAYLOAD'), + }) + ); const body = await collect(formDataToStream(fd, () => {})); @@ -52,11 +55,14 @@ describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { it('should strip bare \\r and bare \\n from blob.type', async () => { const fd = new SpecFormData(); - fd.append('f', makeBlobLike({ - type: 'text/plain\rX-A: 1\nX-B: 2', - name: 'f.txt', - payload: Buffer.from('x'), - })); + fd.append( + 'f', + makeBlobLike({ + type: 'text/plain\rX-A: 1\nX-B: 2', + name: 'f.txt', + payload: Buffer.from('x'), + }) + ); const body = await collect(formDataToStream(fd, () => {})); @@ -66,11 +72,14 @@ describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { it('should preserve legitimate Content-Type values', async () => { const fd = new SpecFormData(); - fd.append('doc', makeBlobLike({ - type: 'application/json; charset=utf-8', - name: 'doc.json', - payload: Buffer.from('{}'), - })); + fd.append( + 'doc', + makeBlobLike({ + type: 'application/json; charset=utf-8', + name: 'doc.json', + payload: Buffer.from('{}'), + }) + ); const body = await collect(formDataToStream(fd, () => {})); @@ -79,11 +88,14 @@ describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { it('should default missing blob.type to application/octet-stream', async () => { const fd = new SpecFormData(); - fd.append('bin', makeBlobLike({ - type: '', - name: 'bin', - payload: Buffer.from([0x00, 0x01]), - })); + fd.append( + 'bin', + makeBlobLike({ + type: '', + name: 'bin', + payload: Buffer.from([0x00, 0x01]), + }) + ); const body = await collect(formDataToStream(fd, () => {})); @@ -92,11 +104,14 @@ describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { it('should escape CRLF and quotes in blob.name (Content-Disposition)', async () => { const fd = new SpecFormData(); - fd.append('up', makeBlobLike({ - type: 'text/plain', - name: 'evil\r\nX-Bad: 1".jpg', - payload: Buffer.from('x'), - })); + fd.append( + 'up', + makeBlobLike({ + type: 'text/plain', + name: 'evil\r\nX-Bad: 1".jpg', + payload: Buffer.from('x'), + }) + ); const body = await collect(formDataToStream(fd, () => {})); @@ -106,16 +121,23 @@ describe('formDataToStream (GHSA-445q-vr5w-6q77 CRLF injection)', () => { it('should report stable contentLength that matches emitted bytes', async () => { const fd = new SpecFormData(); - fd.append('photo', makeBlobLike({ - type: 'image/jpeg\r\nX-Injected: PWNED', - name: 'photo.jpg', - payload: Buffer.from('PAYLOAD'), - })); + fd.append( + 'photo', + makeBlobLike({ + type: 'image/jpeg\r\nX-Injected: PWNED', + name: 'photo.jpg', + payload: Buffer.from('PAYLOAD'), + }) + ); let reportedLength; - const stream = formDataToStream(fd, (h) => { - reportedLength = h['Content-Length']; - }, { boundary: 'test-boundary-abc' }); + const stream = formDataToStream( + fd, + (h) => { + reportedLength = h['Content-Length']; + }, + { boundary: 'test-boundary-abc' } + ); const body = await collect(stream); diff --git a/tests/unit/helpers/shouldBypassProxy.test.js b/tests/unit/helpers/shouldBypassProxy.test.js index a05fffd4..cc7527b7 100644 --- a/tests/unit/helpers/shouldBypassProxy.test.js +++ b/tests/unit/helpers/shouldBypassProxy.test.js @@ -101,7 +101,7 @@ describe('helpers::shouldBypassProxy', () => { expect(shouldBypassProxy('not a url')).toBe(false); }); - it('should bypass proxy for 127.0.0.0/8 subnet when no_proxy contains 127.0.0.1 (GHSA-pmwg-cvhr-8vh7)', () => { + it('should bypass proxy for 127.0.0.0/8 subnet when no_proxy contains 127.0.0.1', () => { setNoProxy('localhost,127.0.0.1,::1'); expect(shouldBypassProxy('http://127.0.0.2:9191/secret')).toBe(true); @@ -166,4 +166,98 @@ describe('helpers::shouldBypassProxy', () => { expect(shouldBypassProxy('http://10.0.0.127:8080/')).toBe(false); expect(shouldBypassProxy('http://200.127.0.1:8080/')).toBe(false); }); + + // IPv4-mapped IPv6 normalization: an attacker (or naive caller) can use the + // IPv4-mapped IPv6 representation of an address (e.g. ::ffff:192.168.1.5) + // to dodge a NO_PROXY policy expressed in IPv4 form, or vice-versa. After + // canonicalising both sides, equivalent addresses compare equal. + describe('IPv4-mapped IPv6 normalization', () => { + it('should bypass via IPv4-mapped IPv6 request when NO_PROXY uses the IPv4 form', () => { + setNoProxy('192.168.1.5'); + + expect(shouldBypassProxy('http://[::ffff:192.168.1.5]/')).toBe(true); + }); + + it('should bypass via Node-normalised IPv4-mapped hex request against an IPv4 NO_PROXY', () => { + // Node's URL parser canonicalises [::ffff:192.168.1.5] → [::ffff:c0a8:105]. + // The hex form must unmap to 192.168.1.5 to match the entry. + setNoProxy('192.168.1.5'); + + expect(shouldBypassProxy('http://[::ffff:c0a8:105]/')).toBe(true); + }); + + it('should bypass via plain IPv4 request when NO_PROXY uses the IPv4-mapped IPv6 dotted form', () => { + setNoProxy('::ffff:192.168.1.5'); + + expect(shouldBypassProxy('http://192.168.1.5/')).toBe(true); + }); + + it('should bypass via plain IPv4 request when NO_PROXY uses the IPv4-mapped IPv6 hex form', () => { + setNoProxy('::ffff:a00:1'); + + expect(shouldBypassProxy('http://10.0.0.1/')).toBe(true); + }); + + it('should bypass via plain IPv4 request when NO_PROXY uses a bracketed IPv4-mapped IPv6 entry', () => { + setNoProxy('[::ffff:192.168.1.5]'); + + expect(shouldBypassProxy('http://192.168.1.5/')).toBe(true); + }); + + it('should treat the uncompressed 0:0:0:0:0:ffff: form as equivalent', () => { + setNoProxy('0:0:0:0:0:ffff:10.0.0.1'); + + expect(shouldBypassProxy('http://10.0.0.1/')).toBe(true); + expect(shouldBypassProxy('http://[::ffff:10.0.0.1]/')).toBe(true); + }); + + it('should treat compressed zero-prefix IPv4-mapped IPv6 dotted forms as equivalent', () => { + for (const entry of [ + '0::ffff:192.168.1.5', + '0:0::ffff:192.168.1.5', + '0:0:0::ffff:192.168.1.5', + '0:0:0:0::ffff:192.168.1.5', + ]) { + setNoProxy(entry); + + expect(shouldBypassProxy('http://192.168.1.5/')).toBe(true); + } + }); + + it('should treat compressed zero-prefix IPv4-mapped IPv6 hex forms as equivalent', () => { + for (const entry of [ + '0::ffff:c0a8:105', + '0:0::ffff:c0a8:105', + '0:0:0::ffff:c0a8:105', + '0:0:0:0::ffff:c0a8:105', + ]) { + setNoProxy(entry); + + expect(shouldBypassProxy('http://192.168.1.5/')).toBe(true); + } + }); + + it('should support compressed bracketed IPv4-mapped IPv6 entries with explicit ports', () => { + setNoProxy('[0:0::ffff:192.168.1.5]:8080'); + + expect(shouldBypassProxy('http://192.168.1.5:8080/')).toBe(true); + expect(shouldBypassProxy('http://192.168.1.5:9090/')).toBe(false); + }); + + it('should NOT cross-match unrelated addresses', () => { + setNoProxy('192.168.1.5'); + + // Different IPv4 address inside an IPv4-mapped form must not bypass. + expect(shouldBypassProxy('http://[::ffff:192.168.1.6]/')).toBe(false); + // Non-mapped IPv6 must not be treated as IPv4. + expect(shouldBypassProxy('http://[2001:db8::1]/')).toBe(false); + }); + + it('should leave non-mapped IPv6 addresses comparing as IPv6', () => { + setNoProxy('2001:db8::1'); + + expect(shouldBypassProxy('http://[2001:db8::1]/')).toBe(true); + expect(shouldBypassProxy('http://[2001:db8::2]/')).toBe(false); + }); + }); }); diff --git a/tests/unit/prototypePollution.test.js b/tests/unit/prototypePollution.test.js index 307c41db..1d9faa1b 100644 --- a/tests/unit/prototypePollution.test.js +++ b/tests/unit/prototypePollution.test.js @@ -5,6 +5,8 @@ import http from 'http'; import utils from '../../lib/utils.js'; import mergeConfig from '../../lib/core/mergeConfig.js'; import defaults from '../../lib/defaults/index.js'; +import AxiosError from '../../lib/core/AxiosError.js'; +import AxiosHeaders from '../../lib/core/AxiosHeaders.js'; import axios from '../../index.js'; describe('Prototype Pollution Protection', () => { @@ -47,6 +49,16 @@ describe('Prototype Pollution Protection', () => { delete Object.prototype.withCredentials; delete Object.prototype.responseType; delete Object.prototype.fetchOptions; + delete Object.prototype.username; + delete Object.prototype.password; + delete Object.prototype.hostname; + delete Object.prototype.host; + delete Object.prototype.port; + delete Object.prototype.protocol; + delete Object.prototype.get; + delete Object.prototype.set; + delete Object.prototype.headers; + delete Object.prototype.customNested; }); describe('utils.merge', () => { @@ -247,7 +259,7 @@ describe('Prototype Pollution Protection', () => { assert.strictEqual(result.headers.common['Content-Type'], 'application/json'); }); - // GHSA-pf86-5x62-jrwf gadget 3: polluted transformRequest/Response must not + // Polluted transformRequest/Response must not // replace the safe defaults through inherited reads during merge. it('should not inherit polluted transformRequest from Object.prototype', () => { const polluted = () => 'attacker'; @@ -270,7 +282,7 @@ describe('Prototype Pollution Protection', () => { }); }); - // GHSA-pf86-5x62-jrwf gadget 1: parseReviver read via prototype chain. + // parseReviver read via prototype chain. describe('defaults.transformResponse parseReviver', () => { it('should ignore Object.prototype.parseReviver when parsing JSON', () => { let reviverCalled = false; @@ -281,10 +293,7 @@ describe('Prototype Pollution Protection', () => { }; const ctx = { transitional: defaults.transitional }; - const result = defaults.transformResponse[0].call( - ctx, - '{"role":"user","balance":100}' - ); + const result = defaults.transformResponse[0].call(ctx, '{"role":"user","balance":100}'); assert.strictEqual(reviverCalled, false); assert.strictEqual(result.role, 'user'); @@ -302,9 +311,9 @@ describe('Prototype Pollution Protection', () => { }); }); - // GHSA-w9j2-pvgh-6h63: mergeDirectKeys must not inherit validateStatus from + // mergeDirectKeys must not inherit validateStatus from // Object.prototype (was using the `in` operator which traverses the chain). - describe('GHSA-w9j2-pvgh-6h63 validateStatus merge', () => { + describe('validateStatus merge', () => { it('should not inherit a polluted validateStatus during mergeConfig', () => { Object.prototype.validateStatus = () => true; @@ -339,9 +348,9 @@ describe('Prototype Pollution Protection', () => { }, 10000); }); - // GHSA-3w6x-2g7m-8v23: end-to-end check that a polluted parseReviver does not + // end-to-end check that a polluted parseReviver does not // tamper with JSON response bodies through the full axios.get pipeline. - describe('GHSA-3w6x-2g7m-8v23 parseReviver end-to-end', () => { + describe('parseReviver end-to-end', () => { it('should not let Object.prototype.parseReviver tamper with JSON responses', async () => { let reviverCalled = false; const stolen = {}; @@ -382,7 +391,7 @@ describe('Prototype Pollution Protection', () => { }, 10000); }); - // GHSA-pf86-5x62-jrwf gadget 2: http adapter must not read config.transport + // http adapter must not read config.transport // (or related keys) from Object.prototype. describe('http adapter prototype reads', () => { it('should not invoke Object.prototype.transport on a request', async () => { @@ -412,17 +421,20 @@ describe('Prototype Pollution Protection', () => { }, 10000); }); - // GHSA-q8qp-cvcw-x6jj: five config properties were read via direct property + // Five config properties were read via direct property // access in the http adapter and resolveConfig, bypassing hasOwnProperty and // allowing prototype pollution gadgets (auth, baseURL, socketPath, // beforeRedirect, insecureHTTPParser). - describe('GHSA-q8qp-cvcw-x6jj http adapter gadgets', () => { + describe('http adapter gadgets', () => { function startServer(handler) { return new Promise((resolve) => { - const server = http.createServer(handler || ((req, res) => { - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ headers: req.headers, url: req.url })); - })); + const server = http.createServer( + handler || + ((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ headers: req.headers, url: req.url })); + }) + ); server.listen(0, '127.0.0.1', () => resolve(server)); }); } @@ -531,18 +543,72 @@ describe('Prototype Pollution Protection', () => { // HPE_CR_EXPECTED). Match any parser error to remain stable across // Node releases while still confirming the strict parser rejected // the payload. - assert.match( - caughtCode, - /^HPE_/, - `expected an HPE_* parser error, got: ${caughtCode}` - ); + assert.match(caughtCode, /^HPE_/, `expected an HPE_* parser error, got: ${caughtCode}`); } finally { await new Promise((resolve) => malformed.close(resolve)); } }, 10000); + + it('should not inject Proxy-Authorization from polluted Object.prototype.auth', async () => { + // setProxy reads `proxy.auth` directly. When `proxy` is a + // URL instance from the environment proxy or a plain object without an own `auth`, + // a polluted Object.prototype.auth would otherwise be base64-encoded into the + // Proxy-Authorization header, leaking attacker-controlled credentials. + Object.prototype.auth = { username: 'attacker', password: 'exfil' }; + + const proxy = await startServer(); + const { port: proxyPort } = proxy.address(); + + const target = await startServer(); + const { port: targetPort } = target.address(); + + try { + const res = await axios.get(`http://127.0.0.1:${targetPort}/api`, { + proxy: { host: '127.0.0.1', port: proxyPort, protocol: 'http' }, + }); + assert.strictEqual(res.status, 200); + assert.strictEqual( + res.data.headers['proxy-authorization'], + undefined, + 'polluted Object.prototype.auth must not produce a Proxy-Authorization header' + ); + } finally { + await stopServer(target); + await stopServer(proxy); + } + }, 10000); + + it('should not inject Proxy-Authorization from polluted Object.prototype.username', async () => { + // The setProxy username/password branch builds basic creds from `proxy.username` + // and `proxy.password`. For a plain object proxy, both reads must be guarded + // against prototype pollution. + Object.prototype.username = 'attacker'; + Object.prototype.password = 'exfil'; + + const proxy = await startServer(); + const { port: proxyPort } = proxy.address(); + + const target = await startServer(); + const { port: targetPort } = target.address(); + + try { + const res = await axios.get(`http://127.0.0.1:${targetPort}/api`, { + proxy: { host: '127.0.0.1', port: proxyPort, protocol: 'http' }, + }); + assert.strictEqual(res.status, 200); + assert.strictEqual( + res.data.headers['proxy-authorization'], + undefined, + 'polluted Object.prototype.username must not produce a Proxy-Authorization header' + ); + } finally { + await stopServer(target); + await stopServer(proxy); + } + }, 10000); }); - describe('GHSA-q8qp-cvcw-x6jj resolveConfig baseURL gadget', () => { + describe('resolveConfig baseURL gadget', () => { // The baseURL branch in buildFullPath only runs when the requested URL is // relative (or allowAbsoluteUrls === false). An absolute URL would skip // baseURL regardless of pollution and would not exercise the gadget. We @@ -658,24 +724,29 @@ describe('Prototype Pollution Protection', () => { }); }); - // Verify every gadget enumerated in the audit (extension of GHSA-q8qp-cvcw-x6jj) + // Verify every gadget enumerated in the audit // is neutralized end-to-end by the null-prototype config. describe('Full gadget coverage via null-prototype config', () => { function startEcho(handler) { return new Promise((resolve) => { - const server = http.createServer(handler || ((req, res) => { - let body = ''; - req.on('data', (c) => (body += c)); - req.on('end', () => { - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ - url: req.url, - method: req.method, - headers: req.headers, - body, - })); - }); - })); + const server = http.createServer( + handler || + ((req, res) => { + let body = ''; + req.on('data', (c) => (body += c)); + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end( + JSON.stringify({ + url: req.url, + method: req.method, + headers: req.headers, + body, + }) + ); + }); + }) + ); server.listen(0, '127.0.0.1', () => resolve(server)); }); } @@ -721,7 +792,14 @@ describe('Prototype Pollution Protection', () => { let hijacked = false; Object.prototype.adapter = function pollutedAdapter() { hijacked = true; - return Promise.resolve({ data: 'pwned', status: 200, statusText: 'OK', headers: {}, config: {}, request: {} }); + return Promise.resolve({ + data: 'pwned', + status: 200, + statusText: 'OK', + headers: {}, + config: {}, + request: {}, + }); }; const server = await startEcho(); @@ -896,4 +974,210 @@ describe('Prototype Pollution Protection', () => { } }, 10000); }); + + // utils.merge previously read `result[targetKey]` directly, which walks the + // prototype chain. A polluted Object.prototype. object would surface as + // the existing value and be merged into the result. + describe('utils.merge prototype-chain read', () => { + it('should not pick up polluted Object.prototype. as the existing value', () => { + Object.prototype.headers = { evil: 'yes' }; + + const result = utils.merge({}, { headers: { 'Content-Type': 'application/json' } }); + + assert.strictEqual(result.headers.evil, undefined); + assert.strictEqual(result.headers['Content-Type'], 'application/json'); + }); + + it('should not absorb polluted nested objects when the key is absent from inputs', () => { + // When the source does not carry `customNested`, the merged result should + // not surface it either, even if Object.prototype carries it. + Object.prototype.customNested = { evil: 'yes' }; + + const result = utils.merge({}, { safe: 'value' }); + + assert.strictEqual(result.hasOwnProperty('customNested'), false); + assert.strictEqual(result.safe, 'value'); + }); + }); + + // Object.defineProperty calls a HasProperty check on `get`/`set` of the + // descriptor. A polluted Object.prototype.get with a non-function value would + // throw TypeError at every defineProperty site that uses a plain literal + // descriptor. Each fixed site should be shielded with `__proto__: null`. + describe('Object.defineProperty descriptor literals', () => { + it('should construct AxiosError when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + const err = new AxiosError('hello', 'ECODE'); + + assert.strictEqual(err.message, 'hello'); + assert.strictEqual(err.code, 'ECODE'); + }); + + it('should construct AxiosHeaders accessor methods when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + // AxiosHeaders.accessor uses Object.defineProperty on the prototype. + // Triggering a fresh accessor definition exercises the descriptor literal. + AxiosHeaders.accessor('X-Pp-Test'); + + const h = new AxiosHeaders(); + h.setXPpTest('value'); + assert.strictEqual(h.getXPpTest(), 'value'); + }); + + it('should not throw in mergeConfig when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + const result = mergeConfig({}, { url: '/x', method: 'get' }); + + assert.strictEqual(result.url, '/x'); + assert.strictEqual(result.method, 'get'); + assert.strictEqual(typeof result.hasOwnProperty, 'function'); + }); + + it('should not throw in utils.extend when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + const a = {}; + const b = { x: 1, fn() {} }; + utils.extend(a, b); + + assert.strictEqual(a.x, 1); + assert.strictEqual(typeof a.fn, 'function'); + }); + + it('should not throw in utils.extend with thisArg when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + const a = {}; + const ctx = { tag: 'ctx' }; + const b = { + method() { + return this.tag; + }, + }; + utils.extend(a, b, ctx); + + assert.strictEqual(a.method(), 'ctx'); + }); + + it('should not throw in utils.inherits when Object.prototype.get is polluted', () => { + Object.prototype.get = 'attacker'; + + function Parent() {} + function Child() {} + utils.inherits(Child, Parent); + + assert.strictEqual(Child.prototype.constructor, Child); + assert.strictEqual(Child.super, Parent.prototype); + }); + + it('should also be shielded against a polluted Object.prototype.set', () => { + Object.prototype.set = 'attacker'; + + // Same surface as `get` — ToPropertyDescriptor checks both. One spot-check + // covers them all since they share the same fix. + const err = new AxiosError('hello'); + assert.strictEqual(err.message, 'hello'); + }); + }); + + // End-to-end regressions covering published advisory PoCs against full axios + // request flow. Each test mirrors the exploit scenario from the advisory and + // asserts the attack does not succeed. + describe('advisory regression — full request flow', () => { + function startServer(handler) { + return new Promise((resolve) => { + const server = http.createServer(handler); + server.listen(0, '127.0.0.1', () => resolve(server)); + }); + } + const stop = (s) => new Promise((r) => s.close(r)); + + // Full MITM via prototype pollution gadget in + // `config.proxy`. mergeConfig must not surface a polluted Object.prototype.proxy + // as the merged config's proxy, otherwise every request would route through + // an attacker-controlled host. + it('polluted Object.prototype.proxy must not redirect requests through an attacker proxy', async () => { + const proxyHits = []; + const attackerProxy = await startServer((req, res) => { + proxyHits.push({ + url: req.url, + authorization: req.headers.authorization, + host: req.headers.host, + }); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"hijacked":true}'); + }); + + const realHits = []; + const realServer = await startServer((req, res) => { + realHits.push({ url: req.url }); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"data":"real"}'); + }); + + try { + Object.prototype.proxy = { + protocol: 'http', + host: '127.0.0.1', + port: attackerProxy.address().port, + }; + + const realPort = realServer.address().port; + const res = await axios.get(`http://127.0.0.1:${realPort}/api/secrets`, { + auth: { username: 'admin', password: 'SuperSecret123!' }, + }); + + assert.strictEqual(proxyHits.length, 0, 'attacker proxy must not receive any request'); + assert.strictEqual(realHits.length, 1, 'request must reach the real target'); + assert.deepStrictEqual(res.data, { data: 'real' }); + } finally { + await stop(attackerProxy); + await stop(realServer); + } + }, 10000); + + // Credential theft and response hijacking via + // prototype pollution gadget in config merge. A polluted + // Object.prototype.transformResponse function would otherwise execute with + // `this = config`, exposing `auth.username`/`auth.password` to the attacker. + it('polluted Object.prototype.transformResponse must not be invoked or leak request credentials', async () => { + let invoked = false; + let stolen = null; + Object.prototype.transformResponse = function pollutedTransform(data) { + invoked = true; + stolen = { + url: this && this.url, + username: this && this.auth && this.auth.username, + password: this && this.auth && this.auth.password, + data, + }; + return true; + }; + + const server = await startServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"secret":"keep-me"}'); + }); + + try { + const { port } = server.address(); + const res = await axios.get(`http://127.0.0.1:${port}/users`, { + auth: { username: 'svc-account', password: 'prod-secret-key-123!' }, + }); + + assert.strictEqual(invoked, false, 'polluted transformResponse must not run'); + assert.strictEqual(stolen, null, 'no request context must be captured'); + assert.deepStrictEqual( + res.data, + { secret: 'keep-me' }, + 'response data must reach the caller untampered' + ); + } finally { + await stop(server); + } + }, 10000); + }); });