CVE-2022-1650: Improper Removal of Sensitive Information Before Storage or Transfer (CWE-212)
value: Improper Removal of Sensitive Information Before Storage or Transfer in GitHub repository eventsource/eventsource prior to v2.0.2.


CWE description: The product stores, transfers, or shares a resource that contains sensitive information, but it does not properly remove that information before the product makes the resource available to unauthorized actors.
CVSS v3 base score: 9.3
Published: 2022-05-12T11:15Z
Repository: https://github.com/eventsource/eventsource
Language: JavaScript
Files changed: lib/eventsource.js, test/eventsource_test.js
Commit date: 2022-05-12 06:39:46 +0200
Commit message: fix: strip sensitive headers on redirect to different origin

Diff: diff --git a/lib/eventsource.js b/lib/eventsource.js
index ffcbe37..d6836bb 100644
--- a/lib/eventsource.js
+++ b/lib/eventsource.js
@@ -30,10 +30,12 @@ function hasBom (buf) {
  * @param {Object} [eventSourceInitDict] extra init params. See README for details.
  * @api public
  **/
 function EventSource (url, eventSourceInitDict) {
   var readyState = EventSource.CONNECTING
+  var headers = eventSourceInitDict && eventSourceInitDict.headers
+  var hasNewOrigin = false
   Object.defineProperty(this, 'readyState', {
     get: function () {
       return readyState
     }
   })
@@ -51,15 +53,16 @@ function EventSource (url, eventSourceInitDict) {
   function onConnectionClosed (message) {
     if (readyState === EventSource.CLOSED) return
     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) {
         return
       }
@@ -68,13 +71,13 @@ function EventSource (url, eventSourceInitDict) {
     }, self.reconnectInterval)
   }
 
   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
   var data = ''
   var eventName = ''
@@ -84,13 +87,14 @@ function EventSource (url, eventSourceInitDict) {
   function connect () {
     var options = parse(url)
     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
         }
       }
     }
@@ -146,17 +150,21 @@ function EventSource (url, eventSourceInitDict) {
         return
       }
 
       // 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
       }
 
       if (res.statusCode !== 200) {
@@ -461,5 +469,25 @@ function MessageEvent (type, eventInitDict) {
     if (eventInitDict.hasOwnProperty(f)) {
       Object.defineProperty(this, f, { writable: false, value: eventInitDict[f], enumerable: true })
     }
   }
 }
+
+/**
+ * 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
@@ -579,10 +579,53 @@ describe('HTTP Request', function () {
           server.close(done)
         }
       })
     })
 
+    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)
 
         server.on('request', function (req, res) {

Vulnerable code:     // The url may have been changed by a temporary
    // redirect. If that's the case, revert it now.
  if (eventSourceInitDict && eventSourceInitDict.headers && eventSourceInitDict.headers['Last-Event-ID']) {
    lastEventId = eventSourceInitDict.headers['Last-Event-ID']
    delete eventSourceInitDict.headers['Last-Event-ID']
    if (eventSourceInitDict && eventSourceInitDict.headers) {
      for (var i in eventSourceInitDict.headers) {
        var header = eventSourceInitDict.headers[i]
        if (!res.headers.location) {
        url = res.headers.location
Fixed code:   var headers = eventSourceInitDict && eventSourceInitDict.headers
  var hasNewOrigin = false
    // 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
      hasNewOrigin = false
  if (headers && headers['Last-Event-ID']) {
    lastEventId = headers['Last-Event-ID']
    delete headers['Last-Event-ID']
    if (headers) {
      var reqHeaders = hasNewOrigin ? removeUnsafeHeaders(headers) : headers
      for (var i in reqHeaders) {
        var header = reqHeaders[i]
        var location = res.headers.location
        if (!location) {
        var prevOrigin = new URL(url).origin
        var nextOrigin = new URL(location).origin
        hasNewOrigin = prevOrigin !== nextOrigin
        url = location

/**
 * 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
}
    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)
        }
      })
    })