bpo-39503: CVE-2020-8492: Fix AbstractBasicAuthHandler (GH-18284)

The AbstractBasicAuthHandler class of the urllib.request module uses
an inefficient regular expression which can be exploited by an
attacker to cause a denial of service. Fix the regex to prevent the
catastrophic backtracking. Vulnerability reported by Ben Caller
and Matt Schwager.

AbstractBasicAuthHandler of urllib.request now parses all
WWW-Authenticate HTTP headers and accepts multiple challenges per
header: use the realm of the first Basic challenge.

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
This commit is contained in:
Victor Stinner 2020-04-02 02:52:20 +02:00 committed by GitHub
parent d57cf55736
commit 0b297d4ff1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 115 additions and 52 deletions

View File

@ -1446,40 +1446,64 @@ 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 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):
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)
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()

View File

@ -937,8 +937,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"
@ -950,27 +957,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)

View File

@ -0,0 +1,3 @@
:class:`~urllib.request.AbstractBasicAuthHandler` of :mod:`urllib.request`
now parses all WWW-Authenticate HTTP headers and accepts multiple challenges
per header: use the realm of the first Basic challenge.

View File

@ -0,0 +1,5 @@
CVE-2020-8492: The :class:`~urllib.request.AbstractBasicAuthHandler` class of the
:mod:`urllib.request` module uses an inefficient regular expression which can
be exploited by an attacker to cause a denial of service. Fix the regex to
prevent the catastrophic backtracking. Vulnerability reported by Ben Caller
and Matt Schwager.