CVE-2018-25091: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: urllib3 before 1.24.2 does not remove the authorization HTTP header when following a cross-origin redirect (i.e., a redirect that differs in host, port, or scheme). This can allow for credentials in the authorization header to be exposed to unintended hosts or transmitted in cleartext. NOTE: this issue exists because of an incomplete fix for CVE-2018-20060 (which was case-sensitive).
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 6.1
Published: 2023-10-15T19:15Z
Repository: https://github.com/urllib3/urllib3
Language: Python
Files changed: CHANGES.rst, CONTRIBUTORS.txt, src/urllib3/poolmanager.py, src/urllib3/util/retry.py, test/test_retry.py, test/with_dummyserver/test_poolmanager.py
Commit date: 2018-12-29 16:41:54 -0600
Commit message: Remove Authorization headers regardless of case on cross-origin redirects (#1511)



Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index 8ef3f15..4e9a1ab 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -6,10 +6,12 @@ dev (master)
 
 * Implemented a more efficient ``HTTPResponse.__iter__()`` method. (Issue #1483)
 
 * Upgraded ``urllib3.utils.parse_url()`` to be RFC 3986 compliant. (Pull #1487)
 
+* Remove Authorization header regardless of case when redirecting to cross-site. (Issue #1510)
+
 * ... [Short description of non-trivial change.] (Issue #)
 
 
 1.24.1 (2018-11-02)
 -------------------
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 71813f9..02b24fc 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -270,7 +270,10 @@ In chronological order:
   * Bugfix for responses with Content-Type: message/* logging warnings
 
 * Justin Bramley <https://github.com/jbramleycl>
   * Add ability to handle multiple Content-Encodings
 
+* Katsuhiko YOSHIDA <https://github.com/kyoshidajp>
+  * Remove Authorization header regardless of case when redirecting to cross-site
+
 * [Your name or handle] <[email or website]>
   * [Brief summary of your changes]
diff --git a/src/urllib3/poolmanager.py b/src/urllib3/poolmanager.py
index 7039324..a13149b 100644
--- a/src/urllib3/poolmanager.py
+++ b/src/urllib3/poolmanager.py
@@ -5,10 +5,11 @@ import logging
 
 from ._collections import RecentlyUsedContainer
 from .connectionpool import HTTPConnectionPool, HTTPSConnectionPool
 from .connectionpool import port_by_scheme
 from .exceptions import LocationValueError, MaxRetryError, ProxySchemeUnknown
+from .packages import six
 from .packages.six.moves.urllib.parse import urljoin
 from .request import RequestMethods
 from .util.url import parse_url
 from .util.retry import Retry
 
@@ -340,12 +341,14 @@ class PoolManager(RequestMethods):
         # Strip headers marked as unsafe to forward to the redirected location.
         # Check remove_headers_on_redirect to avoid a potential network call within
         # conn.is_same_host() which may use socket.gethostbyname() in the future.
         if (retries.remove_headers_on_redirect
                 and not conn.is_same_host(redirect_location)):
-            for header in retries.remove_headers_on_redirect:
-                kw['headers'].pop(header, None)
+            headers = list(six.iterkeys(kw['headers']))
+            for header in headers:
+                if header.lower() in retries.remove_headers_on_redirect:
+                    kw['headers'].pop(header, None)
 
         try:
             retries = retries.increment(method, url, response=response, _pool=conn)
         except MaxRetryError:
             if retries.raise_on_redirect:
diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py
index e7d0abd..02429ee 100644
--- a/src/urllib3/util/retry.py
+++ b/src/urllib3/util/retry.py
@@ -177,11 +177,12 @@ class Retry(object):
         self.backoff_factor = backoff_factor
         self.raise_on_redirect = raise_on_redirect
         self.raise_on_status = raise_on_status
         self.history = history or tuple()
         self.respect_retry_after_header = respect_retry_after_header
-        self.remove_headers_on_redirect = remove_headers_on_redirect
+        self.remove_headers_on_redirect = frozenset([
+            h.lower() for h in remove_headers_on_redirect])
 
     def new(self, **kw):
         params = dict(
             total=self.total,
             connect=self.connect, read=self.read, redirect=self.redirect, status=self.status,
diff --git a/test/test_retry.py b/test/test_retry.py
index b4119b1..886a221 100644
--- a/test/test_retry.py
+++ b/test/test_retry.py
@@ -251,11 +251,11 @@ class TestRetry(object):
             retry.increment(method='POST', error=error)
 
     def test_retry_default_remove_headers_on_redirect(self):
         retry = Retry()
 
-        assert list(retry.remove_headers_on_redirect) == ['Authorization']
+        assert list(retry.remove_headers_on_redirect) == ['authorization']
 
     def test_retry_set_remove_headers_on_redirect(self):
         retry = Retry(remove_headers_on_redirect=['X-API-Secret'])
 
-        assert list(retry.remove_headers_on_redirect) == ['X-API-Secret']
+        assert list(retry.remove_headers_on_redirect) == ['x-api-secret']
diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py
index 2a13722..de0b917 100644
--- a/test/with_dummyserver/test_poolmanager.py
+++ b/test/with_dummyserver/test_poolmanager.py
@@ -121,10 +121,21 @@ class TestPoolManager(HTTPDummyServerTestCase):
 
         data = json.loads(r.data.decode('utf-8'))
 
         self.assertNotIn('Authorization', data)
 
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'authorization': 'foo'})
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('authorization', data)
+        self.assertNotIn('Authorization', data)
+
     def test_redirect_cross_host_no_remove_headers(self):
         http = PoolManager()
         self.addCleanup(http.clear)
 
         r = http.request('GET', '%s/redirect' % self.base_url,
@@ -153,10 +164,24 @@ class TestPoolManager(HTTPDummyServerTestCase):
         data = json.loads(r.data.decode('utf-8'))
 
         self.assertNotIn('X-API-Secret', data)
         self.assertEqual(data['Authorization'], 'bar')
 
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'x-api-secret': 'foo',
+                                  'authorization': 'bar'},
+                         retries=Retry(remove_headers_on_redirect=['X-API-Secret']))
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('x-api-secret', data)
+        self.assertNotIn('X-API-Secret', data)
+        self.assertEqual(data['Authorization'], 'bar')
+
     def test_raise_on_redirect(self):
         http = PoolManager()
         self.addCleanup(http.clear)
 
         r = http.request('GET', '%s/redirect' % self.base_url,

Vulnerable code:             for header in retries.remove_headers_on_redirect:
                kw['headers'].pop(header, None)
        self.remove_headers_on_redirect = remove_headers_on_redirect
        assert list(retry.remove_headers_on_redirect) == ['Authorization']
        assert list(retry.remove_headers_on_redirect) == ['X-API-Secret']
Fixed code: * Remove Authorization header regardless of case when redirecting to cross-site. (Issue #1510)

* Katsuhiko YOSHIDA <https://github.com/kyoshidajp>
  * Remove Authorization header regardless of case when redirecting to cross-site

from .packages import six
            headers = list(six.iterkeys(kw['headers']))
            for header in headers:
                if header.lower() in retries.remove_headers_on_redirect:
                    kw['headers'].pop(header, None)
        self.remove_headers_on_redirect = frozenset([
            h.lower() for h in remove_headers_on_redirect])
        assert list(retry.remove_headers_on_redirect) == ['authorization']
        assert list(retry.remove_headers_on_redirect) == ['x-api-secret']
        r = http.request('GET', '%s/redirect' % self.base_url,
                         fields={'target': '%s/headers' % self.base_url_alt},
                         headers={'authorization': 'foo'})

        self.assertEqual(r.status, 200)

        data = json.loads(r.data.decode('utf-8'))

        self.assertNotIn('authorization', data)
        self.assertNotIn('Authorization', data)

        r = http.request('GET', '%s/redirect' % self.base_url,
                         fields={'target': '%s/headers' % self.base_url_alt},
                         headers={'x-api-secret': 'foo',
                                  'authorization': 'bar'},
                         retries=Retry(remove_headers_on_redirect=['X-API-Secret']))

        self.assertEqual(r.status, 200)

        data = json.loads(r.data.decode('utf-8'))

        self.assertNotIn('x-api-secret', data)
        self.assertNotIn('X-API-Secret', data)
        self.assertEqual(data['Authorization'], 'bar')