Skip to content

Commit 0b297d4

Browse files
vstinnerserhiy-storchaka
vstinner
and
serhiy-storchaka
authored
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 <[email protected]>
1 parent d57cf55 commit 0b297d4

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
@@ -1446,40 +1446,64 @@ def test_osx_proxy_bypass(self):
14461446
bypass = {'exclude_simple': True, 'exceptions': []}
14471447
self.assertTrue(_proxy_bypass_macosx_sysconf('test', bypass))
14481448

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

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

‎Lib/urllib/request.py‎

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

938938
# allow for double- and single-quoted realm values
939939
# (single quotes are a violation of the RFC, but appear in the wild)
940-
rx = re.compile('(?:.*,)*[ \t]*([^ \t]+)[ \t]+'
941-
'realm=(["\']?)([^"\']*)\\2', re.I)
940+
rx = re.compile('(?:^|,)' # start of the string or ','
941+
'[ \t]*' # optional whitespaces
942+
'([^ \t]+)' # scheme like "Basic"
943+
'[ \t]+' # mandatory whitespaces
944+
# realm=xxx
945+
# realm='xxx'
946+
# realm="xxx"
947+
'realm=(["\']?)([^"\']*)\\2',
948+
re.I)
942949

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

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

959-
if authreq:
960-
scheme = authreq.split()[0]
961-
if scheme.lower() != 'basic':
962-
raise ValueError("AbstractBasicAuthHandler does not"
963-
" support the following scheme: '%s'" %
964-
scheme)
965-
else:
966-
mo = AbstractBasicAuthHandler.rx.search(authreq)
967-
if mo:
968-
scheme, quote, realm = mo.groups()
969-
if quote not in ['"',"'"]:
970-
warnings.warn("Basic Auth Realm was unquoted",
971-
UserWarning, 2)
972-
if scheme.lower() == 'basic':
973-
return self.retry_http_basic_auth(host, req, realm)
988+
unsupported = None
989+
for header in headers:
990+
for scheme, realm in self._parse_realm(header):
991+
if scheme.lower() != 'basic':
992+
unsupported = scheme
993+
continue
994+
995+
if realm is not None:
996+
# Use the first matching Basic challenge.
997+
# Ignore following challenges even if they use the Basic
998+
# scheme.
999+
return self.retry_http_basic_auth(host, req, realm)
1000+
1001+
if unsupported is not None:
1002+
raise ValueError("AbstractBasicAuthHandler does not "
1003+
"support the following scheme: %r"
1004+
% (scheme,))
9741005

9751006
def retry_http_basic_auth(self, host, req, realm):
9761007
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)