code.ossystems Code Review - openembedded-core.git/blob

   1 From 0b297d4ff1c0e4480ad33acae793fbaf4bf015b4 Mon Sep 17 00:00:00 2001
   2 From: Victor Stinner <vstinner@python.org>
   3 Date: Thu, 2 Apr 2020 02:52:20 +0200
   4 Subject: [PATCH] bpo-39503: CVE-2020-8492: Fix AbstractBasicAuthHandler
   5  (GH-18284)
   6
   7 Upstream-Status: Backport
   8 (https://github.com/python/cpython/commit/0b297d4ff1c0e4480ad33acae793fbaf4bf015b4)
   9
  10 CVE: CVE-2020-8492
  11
  12 The AbstractBasicAuthHandler class of the urllib.request module uses
  13 an inefficient regular expression which can be exploited by an
  14 attacker to cause a denial of service. Fix the regex to prevent the
  15 catastrophic backtracking. Vulnerability reported by Ben Caller
  16 and Matt Schwager.
  17
  18 AbstractBasicAuthHandler of urllib.request now parses all
  19 WWW-Authenticate HTTP headers and accepts multiple challenges per
  20 header: use the realm of the first Basic challenge.
  21
  22 Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
  23 Signed-off-by: Trevor Gamblin <trevor.gamblin@windriver.com>
  24 ---
  25  Lib/test/test_urllib2.py                      | 90 ++++++++++++-------
  26  Lib/urllib/request.py                         | 69 ++++++++++----
  27  .../2020-03-25-16-02-16.bpo-39503.YmMbYn.rst  |  3 +
  28  .../2020-01-30-16-15-29.bpo-39503.B299Yq.rst  |  5 ++
  29  4 files changed, 115 insertions(+), 52 deletions(-)
  30  create mode 100644 Misc/NEWS.d/next/Library/2020-03-25-16-02-16.bpo-39503.YmMbYn.rst
  31  create mode 100644 Misc/NEWS.d/next/Security/2020-01-30-16-15-29.bpo-39503.B299Yq.rst
  32
  33 diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
  34 index 8abedaac98..e69ac3e213 100644
  35 --- a/Lib/test/test_urllib2.py
  36 +++ b/Lib/test/test_urllib2.py
  37 @@ -1446,40 +1446,64 @@ class HandlerTests(unittest.TestCase):
  38          bypass = {'exclude_simple': True, 'exceptions': []}
  39          self.assertTrue(_proxy_bypass_macosx_sysconf('test', bypass))
  40
  41 -    def test_basic_auth(self, quote_char='"'):
  42 -        opener = OpenerDirector()
  43 -        password_manager = MockPasswordManager()
  44 -        auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
  45 -        realm = "ACME Widget Store"
  46 -        http_handler = MockHTTPHandler(
  47 -            401, 'WWW-Authenticate: Basic realm=%s%s%s\r\n\r\n' %
  48 -            (quote_char, realm, quote_char))
  49 -        opener.add_handler(auth_handler)
  50 -        opener.add_handler(http_handler)
  51 -        self._test_basic_auth(opener, auth_handler, "Authorization",
  52 -                              realm, http_handler, password_manager,
  53 -                              "http://acme.example.com/protected",
  54 -                              "http://acme.example.com/protected",
  55 -                              )
  56 -
  57 -    def test_basic_auth_with_single_quoted_realm(self):
  58 -        self.test_basic_auth(quote_char="'")
  59 -
  60 -    def test_basic_auth_with_unquoted_realm(self):
  61 -        opener = OpenerDirector()
  62 -        password_manager = MockPasswordManager()
  63 -        auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
  64 -        realm = "ACME Widget Store"
  65 -        http_handler = MockHTTPHandler(
  66 -            401, 'WWW-Authenticate: Basic realm=%s\r\n\r\n' % realm)
  67 -        opener.add_handler(auth_handler)
  68 -        opener.add_handler(http_handler)
  69 -        with self.assertWarns(UserWarning):
  70 +    def check_basic_auth(self, headers, realm):
  71 +        with self.subTest(realm=realm, headers=headers):
  72 +            opener = OpenerDirector()
  73 +            password_manager = MockPasswordManager()
  74 +            auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager)
  75 +            body = '\r\n'.join(headers) + '\r\n\r\n'
  76 +            http_handler = MockHTTPHandler(401, body)
  77 +            opener.add_handler(auth_handler)
  78 +            opener.add_handler(http_handler)
  79              self._test_basic_auth(opener, auth_handler, "Authorization",
  80 -                                realm, http_handler, password_manager,
  81 -                                "http://acme.example.com/protected",
  82 -                                "http://acme.example.com/protected",
  83 -                                )
  84 +                                  realm, http_handler, password_manager,
  85 +                                  "http://acme.example.com/protected",
  86 +                                  "http://acme.example.com/protected")
  87 +
  88 +    def test_basic_auth(self):
  89 +        realm = "realm2@example.com"
  90 +        realm2 = "realm2@example.com"
  91 +        basic = f'Basic realm="{realm}"'
  92 +        basic2 = f'Basic realm="{realm2}"'
  93 +        other_no_realm = 'Otherscheme xxx'
  94 +        digest = (f'Digest realm="{realm2}", '
  95 +                  f'qop="auth, auth-int", '
  96 +                  f'nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", '
  97 +                  f'opaque="5ccc069c403ebaf9f0171e9517f40e41"')
  98 +        for realm_str in (
  99 +            # test "quote" and 'quote'
 100 +            f'Basic realm="{realm}"',
 101 +            f"Basic realm='{realm}'",
 102 +
 103 +            # charset is ignored
 104 +            f'Basic realm="{realm}", charset="UTF-8"',
 105 +
 106 +            # Multiple challenges per header
 107 +            f'{basic}, {basic2}',
 108 +            f'{basic}, {other_no_realm}',
 109 +            f'{other_no_realm}, {basic}',
 110 +            f'{basic}, {digest}',
 111 +            f'{digest}, {basic}',
 112 +        ):
 113 +            headers = [f'WWW-Authenticate: {realm_str}']
 114 +            self.check_basic_auth(headers, realm)
 115 +
 116 +        # no quote: expect a warning
 117 +        with support.check_warnings(("Basic Auth Realm was unquoted",
 118 +                                     UserWarning)):
 119 +            headers = [f'WWW-Authenticate: Basic realm={realm}']
 120 +            self.check_basic_auth(headers, realm)
 121 +
 122 +        # Multiple headers: one challenge per header.
 123 +        # Use the first Basic realm.
 124 +        for challenges in (
 125 +            [basic,  basic2],
 126 +            [basic,  digest],
 127 +            [digest, basic],
 128 +        ):
 129 +            headers = [f'WWW-Authenticate: {challenge}'
 130 +                       for challenge in challenges]
 131 +            self.check_basic_auth(headers, realm)
 132
 133      def test_proxy_basic_auth(self):
 134          opener = OpenerDirector()
 135 diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
 136 index 7fe50535da..2a3d71554f 100644
 137 --- a/Lib/urllib/request.py
 138 +++ b/Lib/urllib/request.py
 139 @@ -937,8 +937,15 @@ class AbstractBasicAuthHandler:
 140
 141      # allow for double- and single-quoted realm values
 142      # (single quotes are a violation of the RFC, but appear in the wild)
 143 -    rx = re.compile('(?:.*,)*[ \t]*([^ \t]+)[ \t]+'
 144 -                    'realm=(["\']?)([^"\']*)\\2', re.I)
 145 +    rx = re.compile('(?:^|,)'   # start of the string or ','
 146 +                    '[ \t]*'    # optional whitespaces
 147 +                    '([^ \t]+)' # scheme like "Basic"
 148 +                    '[ \t]+'    # mandatory whitespaces
 149 +                    # realm=xxx
 150 +                    # realm='xxx'
 151 +                    # realm="xxx"
 152 +                    'realm=(["\']?)([^"\']*)\\2',
 153 +                    re.I)
 154
 155      # XXX could pre-emptively send auth info already accepted (RFC 2617,
 156      # end of section 2, and section 1.2 immediately after "credentials"
 157 @@ -950,27 +957,51 @@ class AbstractBasicAuthHandler:
 158          self.passwd = password_mgr
 159          self.add_password = self.passwd.add_password
 160
 161 +    def _parse_realm(self, header):
 162 +        # parse WWW-Authenticate header: accept multiple challenges per header
 163 +        found_challenge = False
 164 +        for mo in AbstractBasicAuthHandler.rx.finditer(header):
 165 +            scheme, quote, realm = mo.groups()
 166 +            if quote not in ['"', "'"]:
 167 +                warnings.warn("Basic Auth Realm was unquoted",
 168 +                              UserWarning, 3)
 169 +
 170 +            yield (scheme, realm)
 171 +
 172 +            found_challenge = True
 173 +
 174 +        if not found_challenge:
 175 +            if header:
 176 +                scheme = header.split()[0]
 177 +            else:
 178 +                scheme = ''
 179 +            yield (scheme, None)
 180 +
 181      def http_error_auth_reqed(self, authreq, host, req, headers):
 182          # host may be an authority (without userinfo) or a URL with an
 183          # authority
 184 -        # XXX could be multiple headers
 185 -        authreq = headers.get(authreq, None)
 186 +        headers = headers.get_all(authreq)
 187 +        if not headers:
 188 +            # no header found
 189 +            return
 190
 191 -        if authreq:
 192 -            scheme = authreq.split()[0]
 193 -            if scheme.lower() != 'basic':
 194 -                raise ValueError("AbstractBasicAuthHandler does not"
 195 -                                 " support the following scheme: '%s'" %
 196 -                                 scheme)
 197 -            else:
 198 -                mo = AbstractBasicAuthHandler.rx.search(authreq)
 199 -                if mo:
 200 -                    scheme, quote, realm = mo.groups()
 201 -                    if quote not in ['"',"'"]:
 202 -                        warnings.warn("Basic Auth Realm was unquoted",
 203 -                                      UserWarning, 2)
 204 -                    if scheme.lower() == 'basic':
 205 -                        return self.retry_http_basic_auth(host, req, realm)
 206 +        unsupported = None
 207 +        for header in headers:
 208 +            for scheme, realm in self._parse_realm(header):
 209 +                if scheme.lower() != 'basic':
 210 +                    unsupported = scheme
 211 +                    continue
 212 +
 213 +                if realm is not None:
 214 +                    # Use the first matching Basic challenge.
 215 +                    # Ignore following challenges even if they use the Basic
 216 +                    # scheme.
 217 +                    return self.retry_http_basic_auth(host, req, realm)
 218 +
 219 +        if unsupported is not None:
 220 +            raise ValueError("AbstractBasicAuthHandler does not "
 221 +                             "support the following scheme: %r"
 222 +                             % (scheme,))
 223
 224      def retry_http_basic_auth(self, host, req, realm):
 225          user, pw = self.passwd.find_user_password(realm, host)
 226 diff --git a/Misc/NEWS.d/next/Library/2020-03-25-16-02-16.bpo-39503.YmMbYn.rst b/Misc/NEWS.d/next/Library/2020-03-25-16-02-16.bpo-39503.YmMbYn.rst
 227 new file mode 100644
 228 index 0000000000..be80ce79d9
 229 --- /dev/null
 230 +++ b/Misc/NEWS.d/next/Library/2020-03-25-16-02-16.bpo-39503.YmMbYn.rst
 231 @@ -0,0 +1,3 @@
 232 +:class:`~urllib.request.AbstractBasicAuthHandler` of :mod:`urllib.request`
 233 +now parses all WWW-Authenticate HTTP headers and accepts multiple challenges
 234 +per header: use the realm of the first Basic challenge.
 235 diff --git a/Misc/NEWS.d/next/Security/2020-01-30-16-15-29.bpo-39503.B299Yq.rst b/Misc/NEWS.d/next/Security/2020-01-30-16-15-29.bpo-39503.B299Yq.rst
 236 new file mode 100644
 237 index 0000000000..9f2800581c
 238 --- /dev/null
 239 +++ b/Misc/NEWS.d/next/Security/2020-01-30-16-15-29.bpo-39503.B299Yq.rst
 240 @@ -0,0 +1,5 @@
 241 +CVE-2020-8492: The :class:`~urllib.request.AbstractBasicAuthHandler` class of the
 242 +:mod:`urllib.request` module uses an inefficient regular expression which can
 243 +be exploited by an attacker to cause a denial of service. Fix the regex to
 244 +prevent the catastrophic backtracking. Vulnerability reported by Ben Caller
 245 +and Matt Schwager.
 246 --
 247 2.24.1
 248