Skip to content

Commit 550c293

Browse files
committed
fix: do not forward credential headers on cross-origin redirect
When following a redirect to a different origin, urllib reused the caller's options verbatim, re-sending Authorization, Cookie, and Proxy-Authorization (and re-applying the Basic auth from `auth`) to the new origin. Strip these in handleRedirect when the redirect crosses the origin boundary. Same-origin redirects are unchanged.
1 parent b54eda6 commit 550c293

2 files changed

Lines changed: 157 additions & 0 deletions

File tree

lib/urllib.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,37 @@ var TEXT_DATA_TYPES = [
6666

6767
var PROTO_RE = /^https?:\/\//i;
6868

69+
// Credential-bearing headers that must not be forwarded across an origin
70+
// boundary when following a redirect, to avoid leaking them to a third party.
71+
var CROSS_ORIGIN_SENSITIVE_HEADERS = [ 'authorization', 'cookie', 'proxy-authorization' ];
72+
73+
function isCrossOriginRedirect(fromHref, toUrl) {
74+
try {
75+
if (URL) {
76+
return new URL(fromHref).origin !== new URL(toUrl, fromHref).origin;
77+
}
78+
var from = urlutil.parse(fromHref);
79+
var to = urlutil.parse(urlutil.resolve(fromHref, toUrl));
80+
return from.protocol !== to.protocol || from.host !== to.host;
81+
} catch (err) {
82+
return false;
83+
}
84+
}
85+
86+
function cleanCrossOriginHeaders(headers) {
87+
var cleaned = {};
88+
if (headers) {
89+
var names = utility.getOwnEnumerables(headers, true);
90+
for (var i = 0; i < names.length; i++) {
91+
var name = names[i];
92+
if (CROSS_ORIGIN_SENSITIVE_HEADERS.indexOf(name.toLowerCase()) === -1) {
93+
cleaned[name] = headers[name];
94+
}
95+
}
96+
}
97+
return cleaned;
98+
}
99+
69100
// Keep-Alive: timeout=5, max=100
70101
var KEEP_ALIVE_RE = /^timeout=(\d+)/i;
71102

@@ -688,6 +719,12 @@ function requestWithCallback(url, args, callback) {
688719
options.headers.host = null;
689720
args.headers = options.headers;
690721
}
722+
// do not forward credential-bearing headers / auth to a different origin
723+
if (isCrossOriginRedirect(parsedUrl.href, newUrl)) {
724+
args.headers = cleanCrossOriginHeaders(args.headers || options.headers);
725+
args.auth = null;
726+
args.digestAuth = null;
727+
}
691728
// avoid done will be execute in the future change.
692729
var cb = callback;
693730
callback = null;

test/redirect-cross-origin.test.js

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
'use strict';
2+
3+
var assert = require('assert');
4+
var http = require('http');
5+
var urllib = require('../');
6+
7+
// On a cross-origin redirect, credential-bearing request headers
8+
// (Authorization, Cookie, Proxy-Authorization) and the `auth` option must NOT
9+
// be forwarded to the new origin. Same-origin redirects should keep them.
10+
describe('test/redirect-cross-origin.test.js', function() {
11+
var serverA;
12+
var serverB;
13+
var portA;
14+
var portB;
15+
16+
before(function(done) {
17+
serverB = http.createServer(function(req, res) {
18+
res.statusCode = 200;
19+
res.setHeader('Content-Type', 'application/json');
20+
res.end(JSON.stringify(req.headers));
21+
});
22+
serverB.listen(0, function() {
23+
portB = serverB.address().port;
24+
serverA = http.createServer(function(req, res) {
25+
if (req.url === '/start-cross') {
26+
res.statusCode = 302;
27+
res.setHeader('Location', 'http://127.0.0.1:' + portB + '/captured');
28+
return res.end('redirect to B');
29+
}
30+
if (req.url === '/start-same') {
31+
res.statusCode = 302;
32+
res.setHeader('Location', '/captured');
33+
return res.end('redirect to self');
34+
}
35+
res.statusCode = 200;
36+
res.setHeader('Content-Type', 'application/json');
37+
res.end(JSON.stringify(req.headers));
38+
});
39+
serverA.listen(0, function() {
40+
portA = serverA.address().port;
41+
done();
42+
});
43+
});
44+
});
45+
46+
after(function() {
47+
if (serverA) {
48+
serverA.close();
49+
}
50+
if (serverB) {
51+
serverB.close();
52+
}
53+
});
54+
55+
function credentialHeaders() {
56+
return {
57+
Authorization: 'Bearer LIVE-AUTH',
58+
Cookie: 'session=LIVE-COOKIE',
59+
'Proxy-Authorization': 'Bearer proxy-LIVE',
60+
};
61+
}
62+
63+
it('should NOT forward credential headers on cross-origin redirect', function(done) {
64+
urllib.request('http://127.0.0.1:' + portA + '/start-cross', {
65+
followRedirect: true,
66+
headers: credentialHeaders(),
67+
}, function(err, data, res) {
68+
assert(!err);
69+
assert.equal(res.statusCode, 200);
70+
assert.equal(res.requestUrls.length, 2);
71+
var received = JSON.parse(data.toString());
72+
assert.equal(received.authorization, undefined);
73+
assert.equal(received.cookie, undefined);
74+
assert.equal(received['proxy-authorization'], undefined);
75+
done();
76+
});
77+
});
78+
79+
it('should keep credential headers on same-origin redirect', function(done) {
80+
urllib.request('http://127.0.0.1:' + portA + '/start-same', {
81+
followRedirect: true,
82+
headers: credentialHeaders(),
83+
}, function(err, data, res) {
84+
assert(!err);
85+
assert.equal(res.statusCode, 200);
86+
assert.equal(res.requestUrls.length, 2);
87+
var received = JSON.parse(data.toString());
88+
assert.equal(received.authorization, 'Bearer LIVE-AUTH');
89+
assert.equal(received.cookie, 'session=LIVE-COOKIE');
90+
assert.equal(received['proxy-authorization'], 'Bearer proxy-LIVE');
91+
done();
92+
});
93+
});
94+
95+
it('should NOT re-inject Basic auth (options.auth) on cross-origin redirect', function(done) {
96+
urllib.request('http://127.0.0.1:' + portA + '/start-cross', {
97+
followRedirect: true,
98+
auth: 'user:passwd',
99+
}, function(err, data, res) {
100+
assert(!err);
101+
assert.equal(res.statusCode, 200);
102+
var received = JSON.parse(data.toString());
103+
assert.equal(received.authorization, undefined);
104+
done();
105+
});
106+
});
107+
108+
it('should keep Basic auth (options.auth) on same-origin redirect', function(done) {
109+
urllib.request('http://127.0.0.1:' + portA + '/start-same', {
110+
followRedirect: true,
111+
auth: 'user:passwd',
112+
}, function(err, data, res) {
113+
assert(!err);
114+
assert.equal(res.statusCode, 200);
115+
var received = JSON.parse(data.toString());
116+
assert.equal(received.authorization, 'Basic ' + Buffer.from('user:passwd').toString('base64'));
117+
done();
118+
});
119+
});
120+
});

0 commit comments

Comments
 (0)