Skip to content

Commit

Permalink
fix: strip sensitive headers on redirect to different origin
Browse files Browse the repository at this point in the history
  • Loading branch information
rexxars committed May 12, 2022
1 parent a95ba90 commit 10ee0c4
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 10 deletions.
48 changes: 38 additions & 10 deletions lib/eventsource.js
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
43 changes: 43 additions & 0 deletions test/eventsource_test.js
Expand Up @@ -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)
Expand Down

0 comments on commit 10ee0c4

Please sign in to comment.