10000 bpo-39503: CVE-2020-8492: Fix AbstractBasicAuthHandler (GH-18284) (GH… · python/cpython@b57a736 · GitHub
[go: up one dir, main page]
Skip to content

Commit b57a736

Browse files
miss-islingtonmiss-islington
authored
bpo-39503: CVE-2020-8492: Fix AbstractBasicAuthHandler (GH-18284) (GH-19297)
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> Co-authored-by: Victor Stinner <vstinner@python.org> (cherry picked from commit 0b297d4)
1 parent 8e069fc commit b57a736

File tree

4 files changed

+115
-52
lines changed

4 files changed

+115
-52
lines changed

‎Lib/test/test_urllib2.py

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,40 +1445,64 @@ def test_osx_proxy_bypass(self):
14451445
bypass = {'exclude_simple': True, 'exceptions': []}
14461446
self.assertTrue(_proxy_bypass_macosx_sysconf('test', bypass))
14471447

1448-
def test_basic_auth(self, quote_char='"'):
1449-
opener = OpenerDirector()
1450-
password_manager = MockPasswordManager()
1451-
auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
1452-
realm = "ACME Widget Store"
1453-
http_handler = MockHTTPHandler(
1454-
401, 'WWW-Authenticate: Basic realm=%s%s%s\r\n\r\n' %
1455-
(quote_char, realm, quote_char))
1456-
opener.add_handler(auth_handler)
1457-
opener.add_handler(http_handler)
1458-
self._test_basic_auth(opener, auth_handler, "Authorization",
1459-
realm, http_handler, password_manager,
1460-
"http://acme.example.com/protected",
1461-
"http://acme.example.com/protected",
1462-
)
1463-
1464-
def test_basic_auth_with_single_quoted_realm(self):
1465-
self.test_basic_auth(quote_char="'")
1466-
1467-
def test_basic_auth_with_unquoted_realm(self):
1468-
opener = OpenerDirector()
1469-
password_manager = MockPasswordManager()
1470-
auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
1471-
realm = "ACME Widget Store"
1472-
http_handler = MockHTTPHandler(
1473-
401, 'WWW-Authenticate: Basic realm=%s\r\n\r\n' % realm)
1474-
opener.add_handler(auth_handler)
1475-
opener.add_handler(http_handler)
1476-
with self.assertWarns(UserWarning):
1448+
def check_basic_auth(self, headers, realm):
1449+
with self.subTest(realm=realm, headers=headers):
1450+
opener = OpenerDirector()
1451+
password_manager = MockPasswordManager()
1452+
auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
1453+
body = '\r\n'.join(headers) + '\r\n\r\n'
1454+
http_handler = MockHTTPHandler(401, body)
1455+
opener.add_handler(auth_handler)
1456+
opener.add_handler(http_handler)
14771457
self._test_basic_auth(opener, auth_handler, "Authorization",
1478-
realm, http_handler, password_manager,
1479-
"http://acme.example.com/protected",
1480-
"http://acme.example.com/protected",
1481-
)
1458+
realm, http_handler, password_manager,
1459+
"http://acme.example.com/protected",
1460+
"http://acme.example.com/protected")
1461+
1462+
def test_basic_auth(self):
1463+
realm = "realm2@example.com"
1464+
realm2 = "realm2@example.com"
1465+
basic = f'Basic realm="{realm}"'
1466+
basic2 = f'Basic realm="{realm2}"'
1467+
other_no_realm = 'Otherscheme xxx'
1468+
digest = (f'Digest realm="{realm2}", '
1469+
f'qop="auth, auth-int", '
1470+
f'nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", '
1471+
f'opaque="5ccc069c403ebaf9f0171e9517f40e41"')
1472+
for realm_str in (
1473+
# test "quote" and 'quote'
1474+
f'Basic realm="{realm}"',
1475+
f"Basic realm='{realm}'",
1476+
1477+
# charset is ignored
1478+
f'Basic realm="{realm}", charset="UTF-8"',
1479+
1480+
# Multiple challenges per header
1481+
f'{basic}, {basic2}',
1482+
f'{basic}, {other_no_realm}',
1483+
f'{other_no_realm}, {basic}',
1484+
f'{basic}, {digest}',
1485+
f'{digest}, {basic}',
1486+
):
1487+
headers = [f'WWW-Authenticate: {realm_str}']
1488+
self.check_basic_auth(headers, realm)
1489+
1490+
# no quote: expect a warning
1491+
with support.check_warnings(("Basic Auth Realm was unquoted",
1492+
UserWarning)):
1493+
headers = [f'WWW-Authenticate: Basic realm={realm}']
1494+
self.check_basic_auth(headers, realm)
1495+
1496+
# Multiple headers: one challenge per header.
1497+
# Use the first Basic realm.
1498+
for challenges in (
1499+
[basic, basic2],
1500+
[basic, digest],
1501+
[digest, basic],
1502+
):
1503+
headers = [f'WWW-Authenticate: {challenge}'
1504+
for challenge in challenges]
1505+
self.check_basic_auth(headers, realm)
14821506

14831507
def test_proxy_basic_auth(self):
14841508
opener = OpenerDirector()

‎Lib/urllib/request.py

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -944,8 +944,15 @@ class AbstractBasicAuthHandler:
944944

945945
# allow for double- and single-quoted realm values
946946
# (single quotes are a violation of the RFC, but appear in the wild)
947-
rx = re.compile('(?:.*,)*[ \t]*([^ \t]+)[ \t]+'
948-
'realm=(["\']?)([^"\']*)\\2', re.I)
947+
rx = re.compile('(?:^|,)' # start of the string or ','
948+
'[ \t]*' # optional whitespaces
949+
'([^ \t]+)' # scheme like "Basic"
950+
'[ \t]+' # mandatory whitespaces
951+
# realm=xxx
952+
# realm='xxx'
953+
# realm="xxx"
954+
'realm=(["\']?)([^"\']*)\\2',
955+
re.I)
949956

950957
# XXX could pre-emptively send auth info already accepted (RFC 2617,
951958
# end of section 2, and section 1.2 immediately after "credentials"
@@ -957,27 +964,51 @@ def __init__(self, password_mgr=None):
957964
self.passwd = password_mgr
958965
self.add_password = self.passwd.add_password
959966

967+
def _parse_realm(self, header):
968+
# parse WWW-Authenticate header: accept multiple challenges per header
969+
found_challenge = False
970+
for mo in AbstractBasicAuthHandler.rx.finditer(header):
971+
scheme, quote, realm = mo.groups()
972+
if quote not in ['"', "'"]:
973+
warnings.warn("Basic Auth Realm was unquoted",
974+
UserWarning, 3)
975+
976+
yield (scheme, realm)
977+
978+
found_challenge = True
979+
980+
if not found_challenge:
981+
if header:
982+
scheme = header.split()[0]
983+
else:
984+
scheme = ''
985+
yield (scheme, None)
986+
960987
def http_error_auth_reqed(self, authreq, host, req, headers):
961988
# host may be an authority (without userinfo) or a URL with an
962989
# authority
963-
# XXX could be multiple headers
964-
authreq = headers.get(authreq, None)
990+
headers = headers.get_all(authreq)
991+
if not headers:
992+
# no header found
993+
return
965994

966-
if authreq:
967-
scheme = authreq.split()[0]
968-
if scheme.lower() != 'basic':
969-
raise ValueError("AbstractBasicAuthHandler does not"
970-
" support the following scheme: '%s'" %
971-
scheme)
972-
else:
973-
mo = AbstractBasicAuthHandler.rx.search(authreq)
974-
if mo:
975-
scheme, quote, realm = mo.groups()
976-
if quote not in ['"',"'"]:
977-
warnings.warn("Basic Auth Realm was unquoted",
978-
UserWarning, 2)
979-
if scheme.lower() == 'basic':
980-
return self.retry_http_basic_auth(host, req, realm)
995+
unsupported = None
996+
for header in headers:
997+
for scheme, realm in self._parse_realm(header):
998+
if scheme.lower() != 'basic':
999+
unsupported = scheme
1000+
continue
1001+
1002+
if realm is not None:
1003+
# Use the first matching Basic challenge.
1004+
# Ignore following challenges even if they use the Basic
1005+
# scheme.
1006+
return self.retry_http_basic_auth(host, req, realm)
1007+
1008+
if unsupported is not None:
1009+
raise ValueError("AbstractBasicAuthHandler does not "
1010+
"support the following scheme: %r"
1011+
% (scheme,))
9811012

9821013
def retry_http_basic_auth(self, host, req, realm):
9831014
user, pw = self.passwd.find_user_password(realm, host)

‎Misc/NEWS.d/next/Library/2020-03-25-16-02-16.bpo-39503.YmMbYn.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:class:`~urllib.request.AbstractBasicAuthHandler` of :mod:`urllib.request`
2+
now parses all WWW-Authenticate HTTP headers and accepts multiple challenges
3+
per header: use the realm of the first Basic challenge.

‎Misc/NEWS.d/next/Security/2020-01-30-16-15-29.bpo-39503.B299Yq.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
CVE-2020-8492: The :class:`~urllib.request.AbstractBasicAuthHandler` class of the
2+
:mod:`urllib.request` module uses an inefficient regular expression which can
3+
be exploited by an attacker to cause a denial of service. Fix the regex to
4+
prevent the catastrophic backtracking. Vulnerability reported by Ben Caller
5+
and Matt Schwager.

0 commit comments

Comments
 (0)
0