From 9dbd12e14d21f06975543a826dd60e7828391f31 Mon Sep 17 00:00:00 2001 From: JinWang An Date: Mon, 26 Apr 2021 15:35:30 +0900 Subject: [PATCH] [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 --- Lib/test/test_urllib2.py | 89 +++++++++++++++++++++++++--------------- Lib/urllib/request.py | 69 ++++++++++++++++++++++--------- 2 files changed, 107 insertions(+), 51 deletions(-) diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 876fcd41..89743932 100644 --- 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 d38f725d..f8b78f3f 100644 --- 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) -- 2.34.1