Skip to content

Commit

Permalink
backport of #1449 (#1453)
Browse files Browse the repository at this point in the history
* backport of #1449

* bump patch version
  • Loading branch information
jimmywarting committed Jan 16, 2022
1 parent 8fe5c4e commit 1ef4b56
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 11 deletions.
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "node-fetch",
"version": "2.6.6",
"version": "2.6.7",
"description": "A light-weight module that brings window.fetch to node.js",
"main": "lib/index.js",
"browser": "./browser.js",
Expand Down
49 changes: 40 additions & 9 deletions src/index.js
Expand Up @@ -13,16 +13,29 @@ import https from 'https';
import zlib from 'zlib';
import Stream from 'stream';

import Body, { writeToStream, getTotalBytes } from './body';
import Response from './response';
import Headers, { createHeadersLenient } from './headers';
import Request, { getNodeRequestOptions } from './request';
import FetchError from './fetch-error';
import AbortError from './abort-error';
import Body, { writeToStream, getTotalBytes } from './body.js';
import Response from './response.js';
import Headers, { createHeadersLenient } from './headers.js';
import Request, { getNodeRequestOptions } from './request.js';
import FetchError from './fetch-error.js';
import AbortError from './abort-error.js';

import whatwgUrl from 'whatwg-url';

const URL = Url.URL || whatwgUrl.URL;

// fix an issue where "PassThrough", "resolve" aren't a named export for node <10
const PassThrough = Stream.PassThrough;
const resolve_url = Url.resolve;

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)
);
};


/**
* Fetch function
Expand Down Expand Up @@ -109,7 +122,19 @@ export default function fetch(url, opts) {
const location = headers.get('Location');

// HTTP fetch step 5.3
const locationURL = location === null ? null : resolve_url(request.url, location);
let locationURL = null;
try {
locationURL = location === null ? null : new URL(location, request.url).toString();
} catch (err) {
// error here can only be invalid URL in Location: header
// do not throw when options.redirect == manual
// let the user extract the errorneous redirect URL
if (request.redirect !== 'manual') {
reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect'));
finalize();
return;
}
}

// HTTP fetch step 5.5
switch (request.redirect) {
Expand Down Expand Up @@ -154,9 +179,15 @@ export default function fetch(url, opts) {
body: request.body,
signal: request.signal,
timeout: request.timeout,
size: request.size
size: request.size
};

if (!isDomainOrSubdomain(request.url, locationURL)) {
for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) {
requestOpts.headers.delete(name);
}
}

// HTTP-redirect fetch step 9
if (res.statusCode !== 303 && request.body && getTotalBytes(request) === null) {
reject(new FetchError('Cannot follow redirect with body being a readable stream', 'unsupported-redirect'));
Expand Down
7 changes: 6 additions & 1 deletion test/server.js
@@ -1,7 +1,6 @@
import * as http from 'http';
import { parse } from 'url';
import * as zlib from 'zlib';
import * as stream from 'stream';
import { multipart as Multipart } from 'parted';

let convert;
Expand Down Expand Up @@ -66,6 +65,12 @@ export default class TestServer {
}));
}

if (p.startsWith('/redirect-to/3')) {
res.statusCode = p.slice(13, 16);
res.setHeader('Location', p.slice(17));
res.end();
}

if (p === '/gzip') {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
Expand Down
47 changes: 47 additions & 0 deletions test/test.js
Expand Up @@ -1569,6 +1569,53 @@ describe('node-fetch', () => {
});
});

it('should not forward secure headers to 3th party', () => {
return 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'
})
}).then(res => res.json()).then(json => {
const headers = new Headers(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', () => {
return 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'
})
}).then(res => res.json().then(json => {
const headers = new Headers(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('should allow PATCH request', function() {
const url = `${base}inspect`;
const opts = {
Expand Down

1 comment on commit 1ef4b56

@JamieSlome
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimmywarting - just for clarity, does this commit address:

https://huntr.dev/bounties/db31e05b-ff10-4057-81a3-37445bf161cd/

The initial researcher (@Haxatron) is slightly concerned that this does not actually fix the issue, and wants to validate this before going live with the report 👍

Please sign in to comment.