Skip to content

Commit

Permalink
[security] Drop sensitive headers when following redirects (#2013)
Browse files Browse the repository at this point in the history
Do not forward the `Authorization` and `Cookie` headers if the redirect
host is different from the original host.
  • Loading branch information
lpinca committed Feb 7, 2022
1 parent 75fdfa9 commit 6946f5f
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 0 deletions.
39 changes: 39 additions & 0 deletions lib/websocket.js
Expand Up @@ -766,6 +766,45 @@ function initAsClient(websocket, address, protocols, options) {
opts.path = parts[1];
}

if (opts.followRedirects) {
if (websocket._redirects === 0) {
websocket._originalHost = parsedUrl.host;

const headers = options && options.headers;

//
// Shallow copy the user provided options so that headers can be changed
// without mutating the original object.
//
options = { ...options, headers: {} };

if (headers) {
for (const [key, value] of Object.entries(headers)) {
options.headers[key.toLowerCase()] = value;
}
}
} else if (parsedUrl.host !== websocket._originalHost) {
//
// Match curl 7.77.0 behavior and drop the following headers. These
// headers are also dropped when following a redirect to a subdomain.
//
delete opts.headers.authorization;
delete opts.headers.cookie;
delete opts.headers.host;
opts.auth = undefined;
}

//
// Match curl 7.77.0 behavior and make the first `Authorization` header win.
// If the `Authorization` header is set, then there is nothing to do as it
// will take precedence.
//
if (opts.auth && !options.headers.authorization) {
options.headers.authorization =
'Basic ' + Buffer.from(opts.auth).toString('base64');
}
}

let req = (websocket._req = get(opts));

if (opts.timeout) {
Expand Down
113 changes: 113 additions & 0 deletions test/websocket.test.js
Expand Up @@ -1140,6 +1140,119 @@ describe('WebSocket', () => {
ws.on('close', () => done());
});
});

it('uses the first url userinfo when following redirects', (done) => {
const wss = new WebSocket.Server({ noServer: true, path: '/foo' });
const authorization = 'Basic Zm9vOmJhcg==';

server.once('upgrade', (req, socket) => {
socket.end('HTTP/1.1 302 Found\r\nLocation: /foo\r\n\r\n');
server.once('upgrade', (req, socket, head) => {
wss.handleUpgrade(req, socket, head, (ws, req) => {
assert.strictEqual(req.headers.authorization, authorization);
ws.close();
});
});
});

const port = server.address().port;
const ws = new WebSocket(`ws://foo:bar@localhost:${port}`, {
followRedirects: true
});

assert.strictEqual(ws._req.getHeader('Authorization'), authorization);

ws.on('close', (code) => {
assert.strictEqual(code, 1005);
assert.strictEqual(ws.url, `ws://foo:bar@localhost:${port}/foo`);
assert.strictEqual(ws._redirects, 1);

wss.close(done);
});
});

describe('When the redirect host is different', () => {
it('drops the `auth` option', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
const port = wss.address().port;

server.once('upgrade', (req, socket) => {
socket.end(
`HTTP/1.1 302 Found\r\nLocation: ws://localhost:${port}/\r\n\r\n`
);
});

const ws = new WebSocket(`ws://localhost:${server.address().port}`, {
auth: 'foo:bar',
followRedirects: true
});

assert.strictEqual(
ws._req.getHeader('Authorization'),
'Basic Zm9vOmJhcg=='
);

ws.on('close', (code) => {
assert.strictEqual(code, 1005);
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
assert.strictEqual(ws._redirects, 1);

wss.close(done);
});
});

wss.on('connection', (ws, req) => {
assert.strictEqual(req.headers.authorization, undefined);
ws.close();
});
});

it('drops the Authorization, Cookie, and Host headers', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
const port = wss.address().port;

server.once('upgrade', (req, socket) => {
socket.end(
`HTTP/1.1 302 Found\r\nLocation: ws://localhost:${port}/\r\n\r\n`
);
});

const ws = new WebSocket(`ws://localhost:${server.address().port}`, {
headers: {
Authorization: 'Basic Zm9vOmJhcg==',
Cookie: 'foo=bar',
Host: 'foo'
},
followRedirects: true
});

assert.strictEqual(
ws._req.getHeader('Authorization'),
'Basic Zm9vOmJhcg=='
);
assert.strictEqual(ws._req.getHeader('Cookie'), 'foo=bar');
assert.strictEqual(ws._req.getHeader('Host'), 'foo');

ws.on('close', (code) => {
assert.strictEqual(code, 1005);
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
assert.strictEqual(ws._redirects, 1);

wss.close(done);
});
});

wss.on('connection', (ws, req) => {
assert.strictEqual(req.headers.authorization, undefined);
assert.strictEqual(req.headers.cookie, undefined);
assert.strictEqual(
req.headers.host,
`localhost:${wss.address().port}`
);
ws.close();
});
});
});
});

describe('Connection with query string', () => {
Expand Down

0 comments on commit 6946f5f

Please sign in to comment.