python · vstinner · Oct 14, 2019 · tirkarthi · Oct 15, 2019 · vstinner
diff --git 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,24 +482,62 @@ 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]
         for url, hostname, port in str_cases + bytes_cases:
             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
@@ -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:
     <scheme>://<netloc>/<path>?<query>#<fragment>
@@ -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
@@ -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
@@ -0,0 +1,2 @@
+The :mod:`urllib.urlparse` module now rejects invalid IPv6 addresses and
+invalid port numbers when parsing an URL.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Fix :func:`urllib.parse.urlparse` for IPv6 address when user or password
		contains "[" or "]" character.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		The :mod:`urllib.urlparse` module now rejects invalid IPv6 addresses and
		invalid port numbers when parsing an URL.