diff --git a/src/index.js b/src/index.js index 7175467ac..c5d811406 100644 --- a/src/index.js +++ b/src/index.js @@ -21,6 +21,7 @@ import Request, {getNodeRequestOptions} from './request.js'; import {FetchError} from './errors/fetch-error.js'; import {AbortError} from './errors/abort-error.js'; import {isRedirect} from './utils/is-redirect.js'; +import {isDomainOrSubdomain} from './utils/is.js'; import {parseReferrerPolicyFromHeader} from './utils/referrer.js'; export {Headers, Request, Response, FetchError, AbortError, isRedirect}; @@ -188,6 +189,18 @@ export default async function fetch(url, options_) { 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 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')); diff --git a/src/utils/is.js b/src/utils/is.js index 377161ff1..876ab4733 100644 --- a/src/utils/is.js +++ b/src/utils/is.js @@ -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) + ); +}; diff --git a/test/main.js b/test/main.js index c2017087c..b9fb2afaa 100644 --- a/test/main.js +++ b/test/main.js @@ -35,6 +35,7 @@ import ResponseOrig from '../src/response.js'; import Body, {getTotalBytes, extractContentType} from '../src/body.js'; import TestServer from './utils/server.js'; import chaiTimeout from './utils/chai-timeout.js'; +import {isDomainOrSubdomain} from '../src/utils/is.js'; const AbortControllerPolyfill = abortControllerPolyfill.AbortController; 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)', () => { const url = `${base}redirect/no-location`; return fetch(url).then(res => { diff --git a/test/utils/server.js b/test/utils/server.js index 03aeb9d2a..6938d5b8b 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -245,6 +245,12 @@ export default class TestServer { 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') { res.statusCode = 302; res.setHeader('Location', '/inspect');