diff --git a/src/index.js b/src/index.js index 82efc9888..7c4aee87b 100644 --- a/src/index.js +++ b/src/index.js @@ -22,7 +22,7 @@ import {FetchError} from './errors/fetch-error.js'; import {AbortError} from './errors/abort-error.js'; import {isRedirect} from './utils/is-redirect.js'; import {FormData} from 'formdata-polyfill/esm.min.js'; -import {isDomainOrSubdomain} from './utils/is.js'; +import {isDomainOrSubdomain, isSameProtocol} from './utils/is.js'; import {parseReferrerPolicyFromHeader} from './utils/referrer.js'; import { Blob, @@ -203,7 +203,10 @@ export default async function fetch(url, options_) { // 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)) { + // headers will also be ignored when following a redirect to a domain using + // a different protocol. For example, a redirect from "https://foo.com" to "http://foo.com" + // will not forward the sensitive headers + if (!isDomainOrSubdomain(request.url, locationURL) || !isSameProtocol(request.url, locationURL)) { for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) { requestOptions.headers.delete(name); } diff --git a/src/utils/is.js b/src/utils/is.js index 00eb89b0e..f9e467e45 100644 --- a/src/utils/is.js +++ b/src/utils/is.js @@ -71,3 +71,17 @@ export const isDomainOrSubdomain = (destination, original) => { return orig === dest || orig.endsWith(`.${dest}`); }; + +/** + * isSameProtocol reports whether the two provided URLs use the same protocol. + * + * Both domains must already be in canonical form. + * @param {string|URL} original + * @param {string|URL} destination + */ +export const isSameProtocol = (destination, original) => { + const orig = new URL(original).protocol; + const dest = new URL(destination).protocol; + + return orig === dest; +}; diff --git a/test/main.js b/test/main.js index bc323eba1..ec58f8285 100644 --- a/test/main.js +++ b/test/main.js @@ -33,7 +33,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'; +import {isDomainOrSubdomain, isSameProtocol} from '../src/utils/is.js'; const AbortControllerPolyfill = abortControllerPolyfill.AbortController; const encoder = new TextEncoder(); @@ -522,7 +522,7 @@ describe('node-fetch', () => { 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 + // Unsafe headers are not removed 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'); @@ -542,6 +542,39 @@ describe('node-fetch', () => { expect(isDomainOrSubdomain('http://bob.uk.com', 'http://xyz.uk.com')).to.be.false; }); + it('should not forward secure headers to changed protocol', async () => { + const res = await fetch('https://httpbin.org/redirect-to?url=http%3A%2F%2Fhttpbin.org%2Fget&status_code=302', { + 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 downgraded http + 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('isSameProtocol', () => { + // Forwarding headers to same protocol is OK + expect(isSameProtocol('http://a.com', 'http://a.com')).to.be.true; + expect(isSameProtocol('https://a.com', 'https://www.a.com')).to.be.true; + + // Forwarding headers to diff protocol is not OK + expect(isSameProtocol('http://b.com', 'https://b.com')).to.be.false; + expect(isSameProtocol('http://www.a.com', 'https://a.com')).to.be.false; + }); + it('should treat broken redirect as ordinary response (follow)', async () => { const url = `${base}redirect/no-location`; const res = await fetch(url);