From cc733a8cb6c93c04b97301022d90b8d19e7d4d3c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 14 Oct 2019 15:39:46 +0200 Subject: [PATCH] bpo-36338: urllib.urlparse rejects invalid IPv6 addresses * bpo-36338: The urllib.urlparse module now rejects invalid IPv6 addresses and invalid port numbers when parsing an URL. * bpo-33342: Fix urlparse() for IPv6 address with user:password when user and/or password contain "[" and/or "]" characters. --- Lib/test/test_urlparse.py | 62 +++++++++++++++--- Lib/urllib/parse.py | 65 +++++++++++++++---- .../2019-10-15-19-06-10.bpo-33342.OI3ROU.rst | 2 + .../2019-10-14-15-42-18.bpo-36338.Vxqtz6.rst | 2 + 4 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-10-15-19-06-10.bpo-33342.OI3ROU.rst create mode 100644 Misc/NEWS.d/next/Security/2019-10-14-15-42-18.bpo-36338.Vxqtz6.rst diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py index 762500789f73ac..2dffbb8d4fb7e1 100644 --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -450,6 +450,7 @@ def test_urljoins(self): self.checkJoin('a', 'b', 'b') def test_RFC2732(self): + # IPv6 address str_cases = [ ('http://Test.python.org:5432/foo/', 'test.python.org', 5432), ('http://12.34.56.78:5432/foo/', '12.34.56.78', 5432), @@ -481,7 +482,8 @@ def test_RFC2732(self): ('http://[::12.34.56.78]:/foo/', '::12.34.56.78', None), ('http://[::ffff:12.34.56.78]:/foo/', '::ffff:12.34.56.78', None), - ] + ('http://[::]/', '::', None), + ] def _encode(t): return t[0].encode('ascii'), t[1].encode('ascii'), t[2] bytes_cases = [_encode(x) for x in str_cases] @@ -489,16 +491,53 @@ def _encode(t): urlparsed = urllib.parse.urlparse(url) self.assertEqual((urlparsed.hostname, urlparsed.port) , (hostname, port)) + # bpo-36338: reject invalid IPv6 addresses str_cases = [ - 'http://::12.34.56.78]/', - 'http://[::1/foo/', - 'ftp://[::1/foo/bad]/bad', - 'http://[::1/foo/bad]/bad', - 'http://[::ffff:12.34.56.78'] + # invalid IPv6 address + 'http://[abcd:x::]/', + 'http://[::1]example.com/', + # missing "[" or "]" + 'http://::12.34.56.78]/', + 'http://[::1/foo/', + 'ftp://[::1/foo/bad]/bad', + 'http://[::1/foo/bad]/bad', + 'http://[::ffff:12.34.56.78', + # double ":" + 'http://[::1]::80/', + # invalid "[" or "]" characters + 'http://[[::1]]/', + 'http://[::1][]/', + 'http://[::1]:[]/', + 'http://good.com[bad.com]', + 'http://benign.com\\[attacker.com]', + ] bytes_cases = [x.encode('ascii') for x in str_cases] for invalid_url in str_cases + bytes_cases: self.assertRaises(ValueError, urllib.parse.urlparse, invalid_url) + def test_user_passwd(self): + # bpo-33342: username and password with "[" and "]" characters + for username in ("", "user", "[", "user[", "us]er", "us%er"): + for password in ("", "password", "p[ass]word", "]", "pa%%word"): + for url_host, hostname, port in ( + ("localhost", "localhost", None), + ("127.0.0.1", "127.0.0.1", None), + ("[::1]", '::1', None), + ("[::1]:", '::1', None), + ("[::1%scope]", '::1%scope', None), + ("[::1]:443", '::1', 443), + ): + netloc = f"{username}:{password}@{url_host}" + url = f"//{netloc}/path" + with self.subTest(url=url): + urlparsed = urllib.parse.urlparse(url) + self.assertEqual(urlparsed.netloc, netloc) + self.assertEqual(urlparsed.username, username) + self.assertEqual(urlparsed.password, password) + self.assertEqual(urlparsed.hostname, hostname) + self.assertEqual(urlparsed.port, port) + self.assertEqual(urlparsed.path, "/path") + def test_urldefrag(self): str_cases = [ ('http://python.org#frag', 'http://python.org', 'frag'), @@ -531,6 +570,12 @@ def test_urlsplit_scoped_IPv6(self): self.assertEqual(p.hostname, b"fe80::822a:a8ff:fe49:470c%tESt") self.assertEqual(p.netloc, b'[FE80::822a:a8ff:fe49:470c%tESt]:1234') + for invalid_char in ('%', '[', ']'): + invalid_url = f'http://[::1%sco{invalid_char}pe]' + with self.subTest(invalid_url=invalid_url): + with self.assertRaises(ValueError): + urllib.parse.urlparse(invalid_url) + def test_urlsplit_attributes(self): url = "HTTP://WWW.PYTHON.ORG/doc/#frag" p = urllib.parse.urlsplit(url) @@ -620,9 +665,8 @@ def test_urlsplit_attributes(self): # Verify an illegal port raises ValueError url = b"HTTP://WWW.PYTHON.ORG:65536/doc/#frag" - p = urllib.parse.urlsplit(url) - with self.assertRaisesRegex(ValueError, "out of range"): - p.port + with self.assertRaisesRegex(ValueError, "Port out of range 0-65535"): + urllib.parse.urlsplit(url) def test_attributes_bad_port(self): """Check handling of invalid ports.""" diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py index 31fd7e16ee72cf..0d3b3aa18b9c19 100644 --- a/Lib/urllib/parse.py +++ b/Lib/urllib/parse.py @@ -167,13 +167,7 @@ def hostname(self): def port(self): port = self._hostinfo[1] if port is not None: - try: - port = int(port, 10) - except ValueError: - message = f'Port could not be cast to integer value as {port!r}' - raise ValueError(message) from None - if not ( 0 <= port <= 65535): - raise ValueError("Port out of range 0-65535") + port = _validate_port(port) return port @@ -388,17 +382,48 @@ def _splitparams(url): i = url.find(';') return url[:i], url[i+1:] +def _validate_port(port_str): + try: + port = int(port_str, 10) + except ValueError: + message = f'Port could not be cast to integer value as {port_str!r}' + raise ValueError(message) from None + if not (0 <= port <= 65535): + raise ValueError("Port out of range 0-65535: %r" % port_str) + return port + +def _check_ipv6_host(host): + # "[ipv6]" host: ensure that it starts with "[", ends with "]" + # and that ipv6 doesn't contain "[" nor "]". + if not(host.startswith('[') and host.endswith(']')): + return False + + parts = host[1:-1].split('%', 1) + ipv6 = parts[0] + scope = (parts[1] if len(parts) > 1 else None) + + import ipaddress + try: + ipaddress.IPv6Address(ipv6) + except ipaddress.AddressValueError: + return False + + if scope is not None and any(char in '%[]' for char in scope): + return False + + return True + def _splitnetloc(url, start=0): delim = len(url) # position of end of domain part of url, default is end for c in '/?#': # look for delimiters; the order is NOT important wdelim = url.find(c, start) # find first of this delim if wdelim >= 0: # if found delim = min(delim, wdelim) # use earliest delim position - return url[start:delim], url[delim:] # return (domain, rest) + netloc = url[start:delim] + url = url[delim:] + return (netloc, url) -def _checknetloc(netloc): - if not netloc or netloc.isascii(): - return +def _checknetloc_nfkc(netloc): # looking for characters like \u2100 that expand to 'a/c' # IDNA uses NFKC equivalence, so normalize for this check import unicodedata @@ -414,6 +439,21 @@ def _checknetloc(netloc): raise ValueError("netloc '" + netloc + "' contains invalid " + "characters under NFKC normalization") +def _checknetloc(netloc): + # Optimization: skip the check for empty string and ASCII-only strings. + # NFKC normalization has no effect on ASCII strings. + if netloc and not netloc.isascii(): + _checknetloc_nfkc(netloc) + host, port = _splitport(netloc) + # don't validate "user:passwd@" + user_passwd, host = _splituser(host) + if '[' in host or ']' in host: + # "http://[ipv6%scope]:port/path" URL + if not _check_ipv6_host(host): + raise ValueError(f"Invalid network location: {netloc!r}") + if port is not None: + _validate_port(port) + def urlsplit(url, scheme='', allow_fragments=True): """Parse a URL into 5 components: :///?# @@ -439,9 +479,6 @@ def urlsplit(url, scheme='', allow_fragments=True): if url[:2] == '//': netloc, url = _splitnetloc(url, 2) - if (('[' in netloc and ']' not in netloc) or - (']' in netloc and '[' not in netloc)): - raise ValueError("Invalid IPv6 URL") if allow_fragments and '#' in url: url, fragment = url.split('#', 1) if '?' in url: diff --git a/Misc/NEWS.d/next/Library/2019-10-15-19-06-10.bpo-33342.OI3ROU.rst b/Misc/NEWS.d/next/Library/2019-10-15-19-06-10.bpo-33342.OI3ROU.rst new file mode 100644 index 00000000000000..95013bb2b67ae7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-10-15-19-06-10.bpo-33342.OI3ROU.rst @@ -0,0 +1,2 @@ +Fix :func:`urllib.parse.urlparse` for IPv6 address when user or password +contains "[" or "]" character. diff --git a/Misc/NEWS.d/next/Security/2019-10-14-15-42-18.bpo-36338.Vxqtz6.rst b/Misc/NEWS.d/next/Security/2019-10-14-15-42-18.bpo-36338.Vxqtz6.rst new file mode 100644 index 00000000000000..d494d7f49cb5be --- /dev/null +++ b/Misc/NEWS.d/next/Security/2019-10-14-15-42-18.bpo-36338.Vxqtz6.rst @@ -0,0 +1,2 @@ +The :mod:`urllib.urlparse` module now rejects invalid IPv6 addresses and +invalid port numbers when parsing an URL.