Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: strip sensitive headers on redirect to different origin #273

Merged
merged 3 commits into from May 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions HISTORY.md
@@ -1,3 +1,7 @@
# [2.0.2](https://github.com/EventSource/eventsource/compare/v2.0.1...v2.0.2)

* Do not include authorization and cookie headers on redirect to different origin ([#273](https://github.com/EventSource/eventsource/pull/273) Espen Hovlandsdal)

# [2.0.1](https://github.com/EventSource/eventsource/compare/v2.0.0...v2.0.1)

* Fix `URL is not a constructor` error for browser ([#268](https://github.com/EventSource/eventsource/pull/268) Ajinkya Rajput)
Expand All @@ -8,6 +12,10 @@
* Preallocate buffer size when reading data for increased performance with large messages ([#239](https://github.com/EventSource/eventsource/pull/239) Pau Freixes)
* Removed dependency on url-parser. Fixes [CVE-2022-0512](https://www.whitesourcesoftware.com/vulnerability-database/CVE-2022-0512) & [CVE-2022-0691](https://nvd.nist.gov/vuln/detail/CVE-2022-0691) ([#249](https://github.com/EventSource/eventsource/pull/249) Alex Hladin)

# [1.1.1](https://github.com/EventSource/eventsource/compare/v1.1.0...v1.1.1)

* Do not include authorization and cookie headers on redirect to different origin ([#273](https://github.com/EventSource/eventsource/pull/273) Espen Hovlandsdal)

# [1.1.0](https://github.com/EventSource/eventsource/compare/v1.0.7...v1.1.0)

* Improve performance for large messages across many chunks ([#130](https://github.com/EventSource/eventsource/pull/130) Trent Willis)
Expand Down
50 changes: 40 additions & 10 deletions lib/eventsource.js
Expand Up @@ -16,6 +16,8 @@ var lineFeed = 10
var carriageReturn = 13
// Beyond 256KB we could not observe any gain in performance
var maxBufferAheadAllocation = 1024 * 256
// Headers matching the pattern should be removed when redirecting to different origin
var reUnsafeHeader = /^(cookie|authorization)$/i

function hasBom (buf) {
return bom.every(function (charCode, index) {
Expand All @@ -32,6 +34,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 +57,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 +75,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 +91,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 +154,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
rexxars marked this conversation as resolved.
Show resolved Hide resolved
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 +473,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 (reUnsafeHeader.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