[CVE-2020-8492]Fix AbstractBasicAuthHandler

Python 2.7 through 2.7.17, 3.5 through 3.5.9,
3.6 through 3.6.10, 3.7 through 3.7.6, and 3.8 through 3.8.1
allows an HTTP server to conduct Regular Expression Denial
of Service (ReDoS) attacks against a client
because of urllib.request.AbstractBasicAuthHandler
catastrophic backtracking.

Change-Id: I7f0232cba39309a35fc262817286dcdae81f537a
Signed-off-by: JinWang An <jinwang.an@samsung.com>

diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 876fcd4199fb9235c894aea5f5b60baac19542e3..8974393273d0dda8b815e96637588196eba8e02d 100644 (file)
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -1445,40 +1445,65 @@ class HandlerTests(unittest.TestCase):
         bypass = {'exclude_simple': True, 'exceptions': []}
         self.assertTrue(_proxy_bypass_macosx_sysconf('test', bypass))
 
-    def test_basic_auth(self, quote_char='"'):
-        opener = OpenerDirector()
-        password_manager = MockPasswordManager()
-        auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
-        realm = "ACME Widget Store"
-        http_handler = MockHTTPHandler(
-            401, 'WWW-Authenticate: Basic realm=%s%s%s\r\n\r\n' %
-            (quote_char, realm, quote_char))
-        opener.add_handler(auth_handler)
-        opener.add_handler(http_handler)
-        self._test_basic_auth(opener, auth_handler, "Authorization",
-                              realm, http_handler, password_manager,
-                              "http://acme.example.com/protected",
-                              "http://acme.example.com/protected",
-                              )
+    def check_basic_auth(self, headers, realm):
+        with self.subTest(realm=realm, headers=headers):
+            opener = OpenerDirector()
+            password_manager = MockPasswordManager()
+            auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
+            body = '\r\n'.join(headers) + '\r\n\r\n'
+            http_handler = MockHTTPHandler(401, body)
+            opener.add_handler(auth_handler)
+            opener.add_handler(http_handler)
 
-    def test_basic_auth_with_single_quoted_realm(self):
-        self.test_basic_auth(quote_char="'")
-
-    def test_basic_auth_with_unquoted_realm(self):
-        opener = OpenerDirector()
-        password_manager = MockPasswordManager()
-        auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
-        realm = "ACME Widget Store"
-        http_handler = MockHTTPHandler(
-            401, 'WWW-Authenticate: Basic realm=%s\r\n\r\n' % realm)
-        opener.add_handler(auth_handler)
-        opener.add_handler(http_handler)
-        with self.assertWarns(UserWarning):
             self._test_basic_auth(opener, auth_handler, "Authorization",
-                                realm, http_handler, password_manager,
-                                "http://acme.example.com/protected",
-                                "http://acme.example.com/protected",
-                                )
+                                  realm, http_handler, password_manager,
+                                  "http://acme.example.com/protected",
+                                  "http://acme.example.com/protected")
+
+    def test_basic_auth(self):
+        realm = "realm2@example.com"
+        realm2 = "realm2@example.com"
+        basic = f'Basic realm="{realm}"'
+        basic2 = f'Basic realm="{realm2}"'
+        other_no_realm = 'Otherscheme xxx'
+        digest = (f'Digest realm="{realm2}", '
+                  f'qop="auth, auth-int", '
+                  f'nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", '
+                  f'opaque="5ccc069c403ebaf9f0171e9517f40e41"')
+        for realm_str in (
+            # test "quote" and 'quote'
+            f'Basic realm="{realm}"',
+            f"Basic realm='{realm}'",
+
+            # charset is ignored
+            f'Basic realm="{realm}", charset="UTF-8"',
+
+            # Multiple challenges per header
+            f'{basic}, {basic2}',
+            f'{basic}, {other_no_realm}',
+            f'{other_no_realm}, {basic}',
+            f'{basic}, {digest}',
+            f'{digest}, {basic}',
+        ):
+            headers = [f'WWW-Authenticate: {realm_str}']
+            self.check_basic_auth(headers, realm)
+
+        # no quote: expect a warning
+        with support.check_warnings(("Basic Auth Realm was unquoted",
+                                     UserWarning)):
+            headers = [f'WWW-Authenticate: Basic realm={realm}']
+            self.check_basic_auth(headers, realm)
+
+        # Multiple headers: one challenge per header.
+        # Use the first Basic realm.
+        for challenges in (
+            [basic,  basic2],
+            [basic,  digest],
+            [digest, basic],
+        ):
+            headers = [f'WWW-Authenticate: {challenge}'
+                       for challenge in challenges]
+            self.check_basic_auth(headers, realm)
 
     def test_proxy_basic_auth(self):
         opener = OpenerDirector()

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index d38f725d8e9fe62527b05ba086abd7e03ff418ec..f8b78f3ff0b26eb924405ba5d0c5420142969cb6 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -944,8 +944,15 @@ class AbstractBasicAuthHandler:
 
     # allow for double- and single-quoted realm values
     # (single quotes are a violation of the RFC, but appear in the wild)
-    rx = re.compile('(?:.*,)*[ \t]*([^ \t]+)[ \t]+'
-                    'realm=(["\']?)([^"\']*)\\2', re.I)
+    rx = re.compile('(?:^|,)'   # start of the string or ','
+                    '[ \t]*'    # optional whitespaces
+                    '([^ \t]+)' # scheme like "Basic"
+                    '[ \t]+'    # mandatory whitespaces
+                    # realm=xxx
+                    # realm='xxx'
+                    # realm="xxx"
+                    'realm=(["\']?)([^"\']*)\\2',
+                    re.I)
 
     # XXX could pre-emptively send auth info already accepted (RFC 2617,
     # end of section 2, and section 1.2 immediately after "credentials"
@@ -957,27 +964,51 @@ class AbstractBasicAuthHandler:
         self.passwd = password_mgr
         self.add_password = self.passwd.add_password
 
+    def _parse_realm(self, header):
+        # parse WWW-Authenticate header: accept multiple challenges per header
+        found_challenge = False
+        for mo in AbstractBasicAuthHandler.rx.finditer(header):
+            scheme, quote, realm = mo.groups()
+            if quote not in ['"', "'"]:
+                warnings.warn("Basic Auth Realm was unquoted",
+                              UserWarning, 3)
+
+            yield (scheme, realm)
+
+            found_challenge = True
+
+        if not found_challenge:
+            if header:
+                scheme = header.split()[0]
+            else:
+                scheme = ''
+            yield (scheme, None)
+
     def http_error_auth_reqed(self, authreq, host, req, headers):
         # host may be an authority (without userinfo) or a URL with an
         # authority
-        # XXX could be multiple headers
-        authreq = headers.get(authreq, None)
+        headers = headers.get_all(authreq)
+        if not headers:
+            # no header found
+            return
 
-        if authreq:
-            scheme = authreq.split()[0]
-            if scheme.lower() != 'basic':
-                raise ValueError("AbstractBasicAuthHandler does not"
-                                 " support the following scheme: '%s'" %
-                                 scheme)
-            else:
-                mo = AbstractBasicAuthHandler.rx.search(authreq)
-                if mo:
-                    scheme, quote, realm = mo.groups()
-                    if quote not in ['"',"'"]:
-                        warnings.warn("Basic Auth Realm was unquoted",
-                                      UserWarning, 2)
-                    if scheme.lower() == 'basic':
-                        return self.retry_http_basic_auth(host, req, realm)
+        unsupported = None
+        for header in headers:
+            for scheme, realm in self._parse_realm(header):
+                if scheme.lower() != 'basic':
+                    unsupported = scheme
+                    continue
+
+                if realm is not None:
+                    # Use the first matching Basic challenge.
+                    # Ignore following challenges even if they use the Basic
+                    # scheme.
+                    return self.retry_http_basic_auth(host, req, realm)
+
+        if unsupported is not None:
+            raise ValueError("AbstractBasicAuthHandler does not "
+                             "support the following scheme: %r"
+                             % (scheme,))
 
     def retry_http_basic_auth(self, host, req, realm):
         user, pw = self.passwd.find_user_password(realm, host)