mirror of
https://github.com/tenrok/axios.git
synced 2026-06-17 19:21:29 +03:00
fix: clear stale header on redirect when target is no-proxy (#10794)
* fix: clear stale header on redirect when target is no-proxy or a different proxy * fix: testing issues * fix: case sensetivity * fix: feedback from cubic * chore: final fixes after full review pass
This commit is contained in:
+13
-2
@@ -194,7 +194,7 @@ function dispatchBeforeRedirect(options, responseDetails) {
|
||||
*
|
||||
* @returns {http.ClientRequestArgs}
|
||||
*/
|
||||
function setProxy(options, configProxy, location) {
|
||||
function setProxy(options, configProxy, location, isRedirect) {
|
||||
let proxy = configProxy;
|
||||
if (!proxy && proxy !== false) {
|
||||
const proxyUrl = getProxyForUrl(location);
|
||||
@@ -204,6 +204,17 @@ function setProxy(options, configProxy, location) {
|
||||
}
|
||||
}
|
||||
}
|
||||
// On redirect re-invocation, strip any stale Proxy-Authorization header carried
|
||||
// over from the prior request (e.g. new target no longer uses a proxy, or uses
|
||||
// a different proxy). Skip on the initial request so user-supplied headers are
|
||||
// preserved. Header names are case-insensitive, so remove every case variant.
|
||||
if (isRedirect && options.headers) {
|
||||
for (const name of Object.keys(options.headers)) {
|
||||
if (name.toLowerCase() === 'proxy-authorization') {
|
||||
delete options.headers[name];
|
||||
}
|
||||
}
|
||||
}
|
||||
if (proxy) {
|
||||
// Basic proxy authorization
|
||||
if (proxy.username) {
|
||||
@@ -240,7 +251,7 @@ function setProxy(options, configProxy, location) {
|
||||
options.beforeRedirects.proxy = function beforeRedirect(redirectOptions) {
|
||||
// Configure proxy for redirected request, passing the original config proxy to apply
|
||||
// the exact same logic as if the redirected request was performed by axios directly.
|
||||
setProxy(redirectOptions, configProxy, redirectOptions.href);
|
||||
setProxy(redirectOptions, configProxy, redirectOptions.href, true);
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -2307,6 +2307,192 @@ describe('supports http with nodejs', () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe('Proxy-Authorization header leak on redirect (GHSA-j5f8-grm9-p9fc)', () => {
|
||||
it('clears a stale Proxy-Authorization header when redirected request resolves to no proxy (configProxy=false)', () => {
|
||||
const options = {
|
||||
headers: {},
|
||||
beforeRedirects: {},
|
||||
hostname: 'initial.example.com',
|
||||
host: 'initial.example.com',
|
||||
port: 80,
|
||||
};
|
||||
|
||||
__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'),
|
||||
'initial request should carry Proxy-Authorization'
|
||||
);
|
||||
|
||||
// Simulate redirect re-invocation where the redirected request is resolved to no proxy.
|
||||
// This mirrors the beforeRedirects.proxy hook being called with configProxy=false.
|
||||
const redirectOptions = {
|
||||
headers: { ...options.headers },
|
||||
beforeRedirects: {},
|
||||
hostname: 'attacker.example.com',
|
||||
host: 'attacker.example.com',
|
||||
port: 443,
|
||||
};
|
||||
__setProxy(redirectOptions, false, 'https://attacker.example.com/final', true);
|
||||
|
||||
assert.strictEqual(
|
||||
redirectOptions.headers['Proxy-Authorization'],
|
||||
undefined,
|
||||
'stale Proxy-Authorization must be stripped when redirected request no longer uses a proxy'
|
||||
);
|
||||
});
|
||||
|
||||
it('clears a stale Proxy-Authorization header when environment-derived proxy is bypassed on redirect (NO_PROXY)', () => {
|
||||
const originalHttpProxy = process.env.http_proxy;
|
||||
const originalHttpsProxy = process.env.https_proxy;
|
||||
const originalNoProxy = process.env.no_proxy;
|
||||
|
||||
process.env.http_proxy = 'http://user:pass@127.0.0.1:8030';
|
||||
process.env.https_proxy = 'http://user:pass@127.0.0.1:8030';
|
||||
process.env.no_proxy = 'attacker.example.com';
|
||||
|
||||
try {
|
||||
const options = {
|
||||
headers: {},
|
||||
beforeRedirects: {},
|
||||
hostname: 'initial.example.com',
|
||||
host: 'initial.example.com',
|
||||
port: 80,
|
||||
};
|
||||
|
||||
__setProxy(options, undefined, 'http://initial.example.com/start');
|
||||
assert.strictEqual(
|
||||
options.headers['Proxy-Authorization'],
|
||||
'Basic ' + Buffer.from('user:pass', 'utf8').toString('base64'),
|
||||
'initial request should pick up proxy credentials from env'
|
||||
);
|
||||
|
||||
const redirectOptions = {
|
||||
headers: { ...options.headers },
|
||||
beforeRedirects: {},
|
||||
hostname: 'attacker.example.com',
|
||||
host: 'attacker.example.com',
|
||||
port: 443,
|
||||
protocol: 'https:',
|
||||
};
|
||||
__setProxy(redirectOptions, undefined, 'https://attacker.example.com/final', true);
|
||||
|
||||
assert.strictEqual(
|
||||
redirectOptions.headers['Proxy-Authorization'],
|
||||
undefined,
|
||||
'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;
|
||||
}
|
||||
});
|
||||
|
||||
it('replaces Proxy-Authorization when redirect target resolves to a different proxy without credentials', () => {
|
||||
const options = {
|
||||
headers: {},
|
||||
beforeRedirects: {},
|
||||
hostname: 'initial.example.com',
|
||||
host: 'initial.example.com',
|
||||
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');
|
||||
|
||||
const redirectOptions = {
|
||||
headers: { ...options.headers },
|
||||
beforeRedirects: {},
|
||||
hostname: 'second.example.com',
|
||||
host: 'second.example.com',
|
||||
port: 80,
|
||||
};
|
||||
__setProxy(redirectOptions, { host: '127.0.0.2', port: 8031 }, 'http://second.example.com/final', true);
|
||||
|
||||
assert.strictEqual(
|
||||
redirectOptions.headers['Proxy-Authorization'],
|
||||
undefined,
|
||||
'stale credentials from previous proxy must not leak to a new proxy without credentials'
|
||||
);
|
||||
});
|
||||
|
||||
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') },
|
||||
beforeRedirects: {},
|
||||
hostname: 'initial.example.com',
|
||||
host: 'initial.example.com',
|
||||
port: 80,
|
||||
};
|
||||
|
||||
__setProxy(options, false, 'http://initial.example.com/start');
|
||||
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') },
|
||||
beforeRedirects: {},
|
||||
hostname: 'attacker.example.com',
|
||||
host: 'attacker.example.com',
|
||||
port: 443,
|
||||
href: 'https://attacker.example.com/final',
|
||||
};
|
||||
|
||||
options.beforeRedirects.proxy(redirectOptions);
|
||||
|
||||
assert.strictEqual(
|
||||
redirectOptions.headers['Proxy-Authorization'],
|
||||
undefined,
|
||||
'beforeRedirects.proxy hook must strip stale Proxy-Authorization when redirect target has no proxy'
|
||||
);
|
||||
});
|
||||
|
||||
it('preserves a user-supplied Proxy-Authorization header on the initial request when no proxy is configured', () => {
|
||||
const userValue = 'Basic ' + Buffer.from('alice:secret', 'utf8').toString('base64');
|
||||
const options = {
|
||||
headers: { 'Proxy-Authorization': userValue },
|
||||
beforeRedirects: {},
|
||||
hostname: 'example.com',
|
||||
host: 'example.com',
|
||||
port: 80,
|
||||
};
|
||||
|
||||
__setProxy(options, false, 'http://example.com/start');
|
||||
|
||||
assert.strictEqual(
|
||||
options.headers['Proxy-Authorization'],
|
||||
userValue,
|
||||
'user-supplied Proxy-Authorization must not be stripped on the initial request'
|
||||
);
|
||||
});
|
||||
|
||||
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'];
|
||||
|
||||
for (const casing of casings) {
|
||||
const redirectOptions = {
|
||||
headers: { [casing]: staleValue },
|
||||
beforeRedirects: {},
|
||||
hostname: 'attacker.example.com',
|
||||
host: 'attacker.example.com',
|
||||
port: 443,
|
||||
};
|
||||
|
||||
__setProxy(redirectOptions, false, 'https://attacker.example.com/final', true);
|
||||
|
||||
const leaked = Object.keys(redirectOptions.headers).filter(
|
||||
(name) => name.toLowerCase() === 'proxy-authorization'
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
leaked,
|
||||
[],
|
||||
`stale Proxy-Authorization with key "${casing}" must be stripped regardless of casing`
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('should support cancel', async () => {
|
||||
const source = axios.CancelToken.source();
|
||||
|
||||
@@ -2675,30 +2861,53 @@ describe('supports http with nodejs', () => {
|
||||
}
|
||||
|
||||
it('should not merge prototype-polluted getHeaders into outgoing request', async () => {
|
||||
let receivedHeaders;
|
||||
const server = await startHTTPServer(
|
||||
(req, res) => {
|
||||
receivedHeaders = req.headers;
|
||||
res.end('{}');
|
||||
// Use a stub transport rather than a real HTTP server: polluting
|
||||
// Object.prototype in-process can destabilise Node's HTTP server
|
||||
// internals and cause spurious ECONNRESET. The stub captures the final
|
||||
// outgoing headers axios constructs, which is what this test asserts on.
|
||||
let capturedHeaders;
|
||||
const stubTransport = {
|
||||
request(options, handleResponse) {
|
||||
capturedHeaders = { ...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;
|
||||
},
|
||||
{ port: SERVER_PORT }
|
||||
);
|
||||
};
|
||||
|
||||
try {
|
||||
pollute();
|
||||
await axios.post(
|
||||
`http://localhost:${server.address().port}/`,
|
||||
'http://stub.invalid/',
|
||||
{ userId: 42 },
|
||||
{ headers: { 'Authorization': 'Bearer VALID_USER_TOKEN' } }
|
||||
{
|
||||
headers: { 'Authorization': 'Bearer VALID_USER_TOKEN' },
|
||||
transport: stubTransport,
|
||||
maxRedirects: 0,
|
||||
}
|
||||
);
|
||||
} finally {
|
||||
cleanup();
|
||||
await stopHTTPServer(server);
|
||||
}
|
||||
|
||||
assert.ok(receivedHeaders, 'request did not reach server');
|
||||
assert.strictEqual(receivedHeaders['x-injected'], undefined);
|
||||
assert.notStrictEqual(receivedHeaders['authorization'], 'Bearer ATTACKER_TOKEN');
|
||||
assert.ok(capturedHeaders, 'transport was not invoked');
|
||||
assert.strictEqual(capturedHeaders['x-injected'], undefined);
|
||||
assert.notStrictEqual(capturedHeaders['Authorization'], 'Bearer ATTACKER_TOKEN');
|
||||
assert.notStrictEqual(capturedHeaders['authorization'], 'Bearer ATTACKER_TOKEN');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user