fix: strip sensitive headers on redirect to different origin

EventSource · May 12, 2022 · 10ee0c4 · 10ee0c4
1 parent a95ba90
commit 10ee0c4
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 10 deletions.
diff --git 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
@@ -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)