Repository: node-fetch/node-fetch (js)

Issue:
Potential security issue
Hey there!

I belong to an open source security research community, and a member (@ranjit-git) has found an issue, but doesn’t know the best way to disclose it.

If not a hassle, might you kindly add a `SECURITY.md` file with an email, or another contact method? GitHub [recommends](https://docs.github.com/en/code-security/getting-started/adding-a-security-policy-to-your-repository) this best practice to ensure security issues are responsibly disclosed, and it would serve as a simple instruction for security researchers in the future.

Thank you for your consideration, and I look forward to hearing from you!

(cc @huntr-helper)

Pull request:
fix(Headers): don't forward secure headers to 3th party
<!-- Thanks for contributing! -->

## Purpose
Avoid forwarding secure headers such as authorization, www-authenticate, cookie & cookie2 when redirecting to a untrusted site. (so it follows browses origin policy) 
(matches Go [behavior](https://github.com/golang/go/blob/24239120bfbff9ebee8e8c344d9d3a8ce460b686/src/net/http/client.go#L1014))

## Changes
When the location headers redirects to an other hostname, strip away secure request headers meant for the original destination

## Additional
Should patch v2 also (not many can update to esm)
___

<!-- Mark the ones you have done and remove unnecessary ones. Add new tasks that fit (like TODOs). -->
- [ ] I updated ./docs/CHANGELOG.md with a link to this PR or Issue
- [ ] I updated ./docs/v3-UPGRADE-GUIDE
- [ ] I updated readme
- [x] I added unit test(s)

___

<!-- Add `- Fixes #_NUMBER_` line for every PR/Issue this PR solves. Do not comma separate them. -->
- Fixes #1443 


Patch:
diff --git a/src/index.js b/src/index.js
index 7175467..c5d8114 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 377161f..876ab47 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)
+	);
+};


Test patch:
diff --git a/test/main.js b/test/main.js
index c201708..b9fb2af 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 03aeb9d..6938d5b 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');