fix(Headers): don't forward secure headers to 3th party (#1449)
* fix(Headers): don't forward secure headers to 3th party * added more narrow test for isDomainOrSubdomain
This commit is contained in:
parent
f2c3d56375
commit
f5d3cf5e25
13
src/index.js
13
src/index.js
|
@ -21,6 +21,7 @@ import Request, {getNodeRequestOptions} from './request.js';
|
||||||
import {FetchError} from './errors/fetch-error.js';
|
import {FetchError} from './errors/fetch-error.js';
|
||||||
import {AbortError} from './errors/abort-error.js';
|
import {AbortError} from './errors/abort-error.js';
|
||||||
import {isRedirect} from './utils/is-redirect.js';
|
import {isRedirect} from './utils/is-redirect.js';
|
||||||
|
import {isDomainOrSubdomain} from './utils/is.js';
|
||||||
import {parseReferrerPolicyFromHeader} from './utils/referrer.js';
|
import {parseReferrerPolicyFromHeader} from './utils/referrer.js';
|
||||||
|
|
||||||
export {Headers, Request, Response, FetchError, AbortError, isRedirect};
|
export {Headers, Request, Response, FetchError, AbortError, isRedirect};
|
||||||
|
@ -188,6 +189,18 @@ export default async function fetch(url, options_) {
|
||||||
referrerPolicy: request.referrerPolicy
|
referrerPolicy: request.referrerPolicy
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// when forwarding sensitive headers like "Authorization",
|
||||||
|
// "WWW-Authenticate", and "Cookie" to untrusted targets,
|
||||||
|
// headers will be ignored when following a redirect to a domain
|
||||||
|
// that is not a subdomain match or exact match of the initial domain.
|
||||||
|
// For example, a redirect from "foo.com" to either "foo.com" or "sub.foo.com"
|
||||||
|
// will forward the sensitive headers, but a redirect to "bar.com" will not.
|
||||||
|
if (!isDomainOrSubdomain(request.url, locationURL)) {
|
||||||
|
for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) {
|
||||||
|
requestOptions.headers.delete(name);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// HTTP-redirect fetch step 9
|
// HTTP-redirect fetch step 9
|
||||||
if (response_.statusCode !== 303 && request.body && options_.body instanceof Stream.Readable) {
|
if (response_.statusCode !== 303 && request.body && options_.body instanceof Stream.Readable) {
|
||||||
reject(new FetchError('Cannot follow redirect with body being a readable stream', 'unsupported-redirect'));
|
reject(new FetchError('Cannot follow redirect with body being a readable stream', 'unsupported-redirect'));
|
||||||
|
|
|
@ -56,3 +56,20 @@ export const isAbortSignal = object => {
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* isDomainOrSubdomain reports whether sub is a subdomain (or exact match) of
|
||||||
|
* the parent domain.
|
||||||
|
*
|
||||||
|
* Both domains must already be in canonical form.
|
||||||
|
* @param {string|URL} original
|
||||||
|
* @param {string|URL} destination
|
||||||
|
*/
|
||||||
|
export const isDomainOrSubdomain = (destination, original) => {
|
||||||
|
const orig = new URL(original).hostname;
|
||||||
|
const dest = new URL(destination).hostname;
|
||||||
|
|
||||||
|
return orig === dest || (
|
||||||
|
orig[orig.length - dest.length - 1] === '.' && orig.endsWith(dest)
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
61
test/main.js
61
test/main.js
|
@ -35,6 +35,7 @@ import ResponseOrig from '../src/response.js';
|
||||||
import Body, {getTotalBytes, extractContentType} from '../src/body.js';
|
import Body, {getTotalBytes, extractContentType} from '../src/body.js';
|
||||||
import TestServer from './utils/server.js';
|
import TestServer from './utils/server.js';
|
||||||
import chaiTimeout from './utils/chai-timeout.js';
|
import chaiTimeout from './utils/chai-timeout.js';
|
||||||
|
import {isDomainOrSubdomain} from '../src/utils/is.js';
|
||||||
|
|
||||||
const AbortControllerPolyfill = abortControllerPolyfill.AbortController;
|
const AbortControllerPolyfill = abortControllerPolyfill.AbortController;
|
||||||
const encoder = new TextEncoder();
|
const encoder = new TextEncoder();
|
||||||
|
@ -496,6 +497,66 @@ describe('node-fetch', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not forward secure headers to 3th party', async () => {
|
||||||
|
const res = await fetch(`${base}redirect-to/302/https://httpbin.org/get`, {
|
||||||
|
headers: new Headers({
|
||||||
|
cookie: 'gets=removed',
|
||||||
|
cookie2: 'gets=removed',
|
||||||
|
authorization: 'gets=removed',
|
||||||
|
'www-authenticate': 'gets=removed',
|
||||||
|
'other-safe-headers': 'stays',
|
||||||
|
'x-foo': 'bar'
|
||||||
|
})
|
||||||
|
});
|
||||||
|
|
||||||
|
const headers = new Headers((await res.json()).headers);
|
||||||
|
// Safe headers are not removed
|
||||||
|
expect(headers.get('other-safe-headers')).to.equal('stays');
|
||||||
|
expect(headers.get('x-foo')).to.equal('bar');
|
||||||
|
// Unsafe headers should not have been sent to httpbin
|
||||||
|
expect(headers.get('cookie')).to.equal(null);
|
||||||
|
expect(headers.get('cookie2')).to.equal(null);
|
||||||
|
expect(headers.get('www-authenticate')).to.equal(null);
|
||||||
|
expect(headers.get('authorization')).to.equal(null);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should forward secure headers to same host', async () => {
|
||||||
|
const res = await fetch(`${base}redirect-to/302/${base}inspect`, {
|
||||||
|
headers: new Headers({
|
||||||
|
cookie: 'is=cookie',
|
||||||
|
cookie2: 'is=cookie2',
|
||||||
|
authorization: 'is=authorization',
|
||||||
|
'other-safe-headers': 'stays',
|
||||||
|
'www-authenticate': 'is=www-authenticate',
|
||||||
|
'x-foo': 'bar'
|
||||||
|
})
|
||||||
|
});
|
||||||
|
|
||||||
|
const headers = new Headers((await res.json()).headers);
|
||||||
|
// Safe headers are not removed
|
||||||
|
expect(res.url).to.equal(`${base}inspect`);
|
||||||
|
expect(headers.get('other-safe-headers')).to.equal('stays');
|
||||||
|
expect(headers.get('x-foo')).to.equal('bar');
|
||||||
|
// Unsafe headers should not have been sent to httpbin
|
||||||
|
expect(headers.get('cookie')).to.equal('is=cookie');
|
||||||
|
expect(headers.get('cookie2')).to.equal('is=cookie2');
|
||||||
|
expect(headers.get('www-authenticate')).to.equal('is=www-authenticate');
|
||||||
|
expect(headers.get('authorization')).to.equal('is=authorization');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('isDomainOrSubdomain', () => {
|
||||||
|
// Forwarding headers to same (sub)domain are OK
|
||||||
|
expect(isDomainOrSubdomain('http://a.com', 'http://a.com')).to.be.true;
|
||||||
|
expect(isDomainOrSubdomain('http://a.com', 'http://www.a.com')).to.be.true;
|
||||||
|
expect(isDomainOrSubdomain('http://a.com', 'http://foo.bar.a.com')).to.be.true;
|
||||||
|
|
||||||
|
// Forwarding headers to parent domain, another sibling or a totally other domain is not ok
|
||||||
|
expect(isDomainOrSubdomain('http://b.com', 'http://a.com')).to.be.false;
|
||||||
|
expect(isDomainOrSubdomain('http://www.a.com', 'http://a.com')).to.be.false;
|
||||||
|
expect(isDomainOrSubdomain('http://bob.uk.com', 'http://uk.com')).to.be.false;
|
||||||
|
expect(isDomainOrSubdomain('http://bob.uk.com', 'http://xyz.uk.com')).to.be.false;
|
||||||
|
});
|
||||||
|
|
||||||
it('should treat broken redirect as ordinary response (follow)', () => {
|
it('should treat broken redirect as ordinary response (follow)', () => {
|
||||||
const url = `${base}redirect/no-location`;
|
const url = `${base}redirect/no-location`;
|
||||||
return fetch(url).then(res => {
|
return fetch(url).then(res => {
|
||||||
|
|
|
@ -245,6 +245,12 @@ export default class TestServer {
|
||||||
res.end();
|
res.end();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (p.startsWith('/redirect-to/3')) {
|
||||||
|
res.statusCode = p.slice(13, 16);
|
||||||
|
res.setHeader('Location', p.slice(17));
|
||||||
|
res.end();
|
||||||
|
}
|
||||||
|
|
||||||
if (p === '/redirect/302') {
|
if (p === '/redirect/302') {
|
||||||
res.statusCode = 302;
|
res.statusCode = 302;
|
||||||
res.setHeader('Location', '/inspect');
|
res.setHeader('Location', '/inspect');
|
||||||
|
|
Loading…
Reference in New Issue