From 76cd81d60310d65d01f9d7b48a8985d8ab89c8b4 Mon Sep 17 00:00:00 2001 From: Senthil Kumaran Date: Thu, 29 Apr 2021 10:16:50 -0700 Subject: [PATCH] bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. (GH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith Co-authored-by: Serhiy Storchaka --- Doc/library/urllib.parse.rst | 13 +++++++++ Lib/test/test_urlparse.py | 29 +++++++++++++++++++ Lib/urllib/parse.py | 6 ++++ .../2021-04-25-07-46-37.bpo-43882.Jpwx85.rst | 6 ++++ 4 files changed, 54 insertions(+) create mode 100644 Misc/NEWS.d/next/Security/2021-04-25-07-46-37.bpo-43882.Jpwx85.rst From 985ac016373403e8ad41f8d563c4355ffa8d49ff Mon Sep 17 00:00:00 2001 From: Senthil Kumaran Date: Wed, 5 May 2021 15:50:05 -0700 Subject: [PATCH] bpo-43882 Remove the newline, and tab early. From query and fragments. (GH-25921) --- Lib/test/test_urlparse.py | 24 ++++++++++++++++-------- Lib/urllib/parse.py | 8 +++++--- 2 files changed, 21 insertions(+), 11 deletions(-) Backport: * Drop Misc/NEWS.d * urllib.parse -> urlparse * Update hunk context diff --git a/Doc/library/urllib.parse.rst b/Doc/library/urllib.parse.rst index 67c2120819..0aaac56288 100644 --- a/Doc/library/urlparse.rst +++ b/Doc/library/urlparse.rst @@ -312,6 +312,9 @@ or on combining URL components into a URL string. decomposed before parsing, or is not a Unicode string, no error will be raised. + Following the `WHATWG spec`_ that updates RFC 3986, ASCII newline + ``\n``, ``\r`` and tab ``\t`` characters are stripped from the URL. + .. versionadded:: 2.2 .. versionchanged:: 2.5 @@ -320,6 +323,10 @@ or on combining URL components into a URL string. Characters that affect netloc parsing under NFKC normalization will now raise :exc:`ValueError`. + .. versionchanged:: 2.7 security update + ASCII newline and tab characters are stripped from the URL. + +.. _WHATWG spec: https://url.spec.whatwg.org/#concept-basic-url-parser .. function:: urlunsplit(parts) @@ -674,6 +681,10 @@ task isn't already covered by the URL parsing functions above. .. seealso:: + `WHATWG`_ - URL Living standard + Working Group for the URL Standard that defines URLs, domains, IP addresses, the + application/x-www-form-urlencoded format, and their API. + :rfc:`3986` - Uniform Resource Identifiers This is the current standard (STD66). Any changes to urlparse module should conform to this. Certain deviations could be observed, which are @@ -697,6 +708,8 @@ task isn't already covered by the URL parsing functions above. :rfc:`1738` - Uniform Resource Locators (URL) This specifies the formal syntax and semantics of absolute URLs. + +.. _WHATWG: https://url.spec.whatwg.org/ .. _urlparse-result-object: diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -612,6 +612,43 @@ self.assertEqual(p1.params, 'phone-context=+1-914-555') + def test_urlsplit_remove_unsafe_bytes(self): + # Remove ASCII tabs and newlines from input + url = "http\t://www.python\n.org\t/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment" + p = urlparse.urlsplit(url) + self.assertEqual(p.scheme, "http") + self.assertEqual(p.netloc, "www.python.org") + self.assertEqual(p.path, "/javascript:alert('msg')/") + self.assertEqual(p.query, "query=something") + self.assertEqual(p.fragment, "fragment") + self.assertEqual(p.username, None) + self.assertEqual(p.password, None) + self.assertEqual(p.hostname, "www.python.org") + self.assertEqual(p.port, None) + self.assertEqual(p.geturl(), "http://www.python.org/javascript:alert('msg')/?query=something#fragment") + + # Remove ASCII tabs and newlines from input as bytes. + url = b"http\t://www.python\n.org\t/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment" + p = urlparse.urlsplit(url) + self.assertEqual(p.scheme, b"http") + self.assertEqual(p.netloc, b"www.python.org") + self.assertEqual(p.path, b"/javascript:alert('msg')/") + self.assertEqual(p.query, b"query=something") + self.assertEqual(p.fragment, b"fragment") + self.assertEqual(p.username, None) + self.assertEqual(p.password, None) + self.assertEqual(p.hostname, b"www.python.org") + self.assertEqual(p.port, None) + self.assertEqual(p.geturl(), b"http://www.python.org/javascript:alert('msg')/?query=something#fragment") + + # with scheme as cache-key + url = "http://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment" + scheme = "ht\ntp" + for _ in range(2): + p = urlparse.urlsplit(url, scheme=scheme) + self.assertEqual(p.scheme, "http") + self.assertEqual(p.geturl(), "http://www.python.org/javascript:alert('msg')/?query=something#fragment") + def test_attributes_bad_port(self): """Check handling of non-integer ports.""" p = urlparse.urlsplit("http://www.example.net:foo") diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py --- a/Lib/urlparse.py +++ b/Lib/urlparse.py @@ -78,6 +78,9 @@ '0123456789' '+-.') +# Unsafe bytes to be removed per WHATWG spec +_UNSAFE_URL_BYTES_TO_REMOVE = ['\t', '\r', '\n'] + MAX_CACHE_SIZE = 20 _parse_cache = {} @@ -456,6 +456,11 @@ def urlsplit(url, scheme='', allow_fragments=True): Return a 5-tuple: (scheme, netloc, path, query, fragment). Note that we don't break the components up in smaller bits (e.g. netloc is a single string) and we don't expand % escapes.""" + + for b in _UNSAFE_URL_BYTES_TO_REMOVE: + url = url.replace(b, "") + scheme = scheme.replace(b, "") + allow_fragments = bool(allow_fragments) key = url, scheme, allow_fragments, type(url), type(scheme) cached = _parse_cache.get(key, None)