From 91e4c96c2a75b3da40ad36413e92f61c17b04d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Wirtel?= Date: Wed, 6 Feb 2019 09:29:28 +0100 Subject: [PATCH 1/3] bpo-35906: Avoid headers injections in urllib --- Lib/test/test_urlparse.py | 4 ++++ Lib/urllib/parse.py | 5 +++++ .../2019-02-06-09-28-59.bpo-35906.UUf668.rst | 1 + 3 files changed, 10 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-02-06-09-28-59.bpo-35906.UUf668.rst diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py index 9c71be53afd42b..88672e21d7726c 100644 --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -1036,6 +1036,10 @@ def test_splithost(self): self.assertEqual(splithost("//example.net/file#"), ('example.net', '/file#')) + # bpo-35906: # avoid the header injection + self.assertEqual(splithost('//127.0.0.1:1234/?q=HTTP/1.1\r\nHeader: Value'), + ('127.0.0.1:1234', '/?q=HTTP/1.1')) + def test_splituser(self): splituser = urllib.parse._splituser self.assertEqual(splituser('User:Pass@www.python.org:080'), diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py index dc2171144fc8ba..7abae913fff59d 100644 --- a/Lib/urllib/parse.py +++ b/Lib/urllib/parse.py @@ -1006,6 +1006,11 @@ def _splithost(url): host_port, path = match.groups() if path and path[0] != '/': path = '/' + path + + splitted_path = path.splitlines() + if splitted_path: + path = splitted_path[0] + return host_port, path return None, url diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-02-06-09-28-59.bpo-35906.UUf668.rst b/Misc/NEWS.d/next/Core and Builtins/2019-02-06-09-28-59.bpo-35906.UUf668.rst new file mode 100644 index 00000000000000..4b7d4b89a9737f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-02-06-09-28-59.bpo-35906.UUf668.rst @@ -0,0 +1 @@ +Avoid headers injection with urllib.urlopen. From c5ca131d90185d6c7bd329d69d77c91a990ff7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Wirtel?= Date: Wed, 6 Feb 2019 09:48:42 +0100 Subject: [PATCH 2/3] refactor the patch --- Lib/urllib/parse.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py index 7abae913fff59d..5e34a700be31ff 100644 --- a/Lib/urllib/parse.py +++ b/Lib/urllib/parse.py @@ -1004,13 +1004,13 @@ def _splithost(url): match = _hostprog.match(url) if match: host_port, path = match.groups() + + if path: + path = path.splitlines()[0] + if path and path[0] != '/': path = '/' + path - splitted_path = path.splitlines() - if splitted_path: - path = splitted_path[0] - return host_port, path return None, url From 36d2ed223b3b850b514b57269c3fee7a9aa0f1b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Wirtel?= Date: Wed, 6 Feb 2019 10:24:09 +0100 Subject: [PATCH 3/3] limit to 72 colums --- Lib/test/test_urlparse.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py index 88672e21d7726c..3b87705dedb344 100644 --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -1037,7 +1037,8 @@ def test_splithost(self): ('example.net', '/file#')) # bpo-35906: # avoid the header injection - self.assertEqual(splithost('//127.0.0.1:1234/?q=HTTP/1.1\r\nHeader: Value'), + self.assertEqual( + splithost('//127.0.0.1:1234/?q=HTTP/1.1\r\nHeader: Value'), ('127.0.0.1:1234', '/?q=HTTP/1.1')) def test_splituser(self):