From 10ee0c4881a6ba2fe65ec18ed195ac35889583c4 Mon Sep 17 00:00:00 2001 From: Espen Hovlandsdal Date: Wed, 11 May 2022 12:10:49 +0200 Subject: [PATCH] fix: strip sensitive headers on redirect to different origin --- lib/eventsource.js | 48 +++++++++++++++++++++++++++++++--------- test/eventsource_test.js | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/lib/eventsource.js b/lib/eventsource.js index ffcbe37..d6836bb 100644 --- a/lib/eventsource.js +++ b/lib/eventsource.js @@ -32,6 +32,8 @@ function hasBom (buf) { **/ function EventSource (url, eventSourceInitDict) { var readyState = EventSource.CONNECTING + var headers = eventSourceInitDict && eventSourceInitDict.headers + var hasNewOrigin = false Object.defineProperty(this, 'readyState', { get: function () { return readyState @@ -53,11 +55,12 @@ function EventSource (url, eventSourceInitDict) { readyState = EventSource.CONNECTING _emit('error', new Event('error', {message: message})) - // The url may have been changed by a temporary - // redirect. If that's the case, revert it now. + // The url may have been changed by a temporary redirect. If that's the case, + // revert it now, and flag that we are no longer pointing to a new origin if (reconnectUrl) { url = reconnectUrl reconnectUrl = null + hasNewOrigin = false } setTimeout(function () { if (readyState !== EventSource.CONNECTING || self.connectionInProgress) { @@ -70,9 +73,9 @@ function EventSource (url, eventSourceInitDict) { var req var lastEventId = '' - if (eventSourceInitDict && eventSourceInitDict.headers && eventSourceInitDict.headers['Last-Event-ID']) { - lastEventId = eventSourceInitDict.headers['Last-Event-ID'] - delete eventSourceInitDict.headers['Last-Event-ID'] + if (headers && headers['Last-Event-ID']) { + lastEventId = headers['Last-Event-ID'] + delete headers['Last-Event-ID'] } var discardTrailingNewline = false @@ -86,9 +89,10 @@ function EventSource (url, eventSourceInitDict) { var isSecure = options.protocol === 'https:' options.headers = { 'Cache-Control': 'no-cache', 'Accept': 'text/event-stream' } if (lastEventId) options.headers['Last-Event-ID'] = lastEventId - if (eventSourceInitDict && eventSourceInitDict.headers) { - for (var i in eventSourceInitDict.headers) { - var header = eventSourceInitDict.headers[i] + if (headers) { + var reqHeaders = hasNewOrigin ? removeUnsafeHeaders(headers) : headers + for (var i in reqHeaders) { + var header = reqHeaders[i] if (header) { options.headers[i] = header } @@ -148,13 +152,17 @@ function EventSource (url, eventSourceInitDict) { // Handle HTTP redirects if (res.statusCode === 301 || res.statusCode === 302 || res.statusCode === 307) { - if (!res.headers.location) { + var location = res.headers.location + if (!location) { // Server sent redirect response without Location header. _emit('error', new Event('error', {status: res.statusCode, message: res.statusMessage})) return } + var prevOrigin = new URL(url).origin + var nextOrigin = new URL(location).origin + hasNewOrigin = prevOrigin !== nextOrigin if (res.statusCode === 307) reconnectUrl = url - url = res.headers.location + url = location process.nextTick(connect) return } @@ -463,3 +471,23 @@ function MessageEvent (type, eventInitDict) { } } } + +/** + * Returns a new object of headers that does not include any authorization and cookie headers + * + * @param {Object} headers An object of headers ({[headerName]: headerValue}) + * @return {Object} a new object of headers + * @api private + */ +function removeUnsafeHeaders (headers) { + var safe = {} + for (var key in headers) { + if (/^(cookie|authorization)$/i.test(key)) { + continue + } + + safe[key] = headers[key] + } + + return safe +} diff --git a/test/eventsource_test.js b/test/eventsource_test.js index 31f6eea..8900042 100644 --- a/test/eventsource_test.js +++ b/test/eventsource_test.js @@ -581,6 +581,49 @@ describe('HTTP Request', function () { }) }) + it('follows http ' + status + ' redirects, drops sensitive headers on origin change', function (done) { + var redirectSuffix = '/foobar' + var clientRequestedRedirectUrl = false + var receivedHeaders = {} + createServer(function (err, server) { + if (err) return done(err) + + var newServerUrl = server.url.replace('http://localhost', 'http://127.0.0.1') + + server.on('request', function (req, res) { + if (req.url === '/') { + res.writeHead(status, { + 'Connection': 'Close', + 'Location': newServerUrl + redirectSuffix + }) + res.end() + } else if (req.url === redirectSuffix) { + clientRequestedRedirectUrl = true + receivedHeaders = req.headers + res.writeHead(200, {'Content-Type': 'text/event-stream'}) + res.end() + } + }) + + var es = new EventSource(server.url, { + headers: { + keep: 'me', + authorization: 'Bearer someToken', + cookie: 'some-cookie=yep' + } + }) + + es.onopen = function () { + assert.ok(clientRequestedRedirectUrl) + assert.equal(newServerUrl + redirectSuffix, es.url) + assert.equal(receivedHeaders.keep, 'me', 'safe header no longer present') + assert.equal(typeof receivedHeaders.authorization, 'undefined', 'authorization header still present') + assert.equal(typeof receivedHeaders.cookie, 'undefined', 'cookie header still present') + server.close(done) + } + }) + }) + it('causes error event when response is ' + status + ' with missing location', function (done) { createServer(function (err, server) { if (err) return done(err)