[issue36274] http.client cannot send non-ASCII request lines
Karthikeyan Singaravelan
report at bugs.python.org
Wed Sep 18 04:42:16 EDT 2019
Karthikeyan Singaravelan <tir.karthi at gmail.com> added the comment:
Is there a reason why cherrypy doesn't URL encode the null byte in this test as per comment : https://github.com/cherrypy/cherrypy/issues/1781#issuecomment-504476658 ?
The original fix was adopted from golang (https://github.com/golang/go/commit/829c5df58694b3345cb5ea41206783c8ccf5c3ca) and as per the cherrypy test the golang client also should be forbidding invalid control character : https://play.golang.org/p/FTNtRmvlWrn
Also as per the discussion one of our own stdlib tests were depending on this behavior in the http client and had to be changed : https://github.com/python/cpython/pull/12755#issuecomment-481617878
The change made in Issue30458 also affects urllib3 test in 3.7.4 causing CI failure and the related discussion to encode URLs : https://github.com/urllib3/urllib3/pull/1671#discussion_r318804155 . urllib3 issue : https://github.com/urllib3/urllib3/issues/1664 .
IIRC urllib3 was also patched for this vulnerability in similar manner as part of larger refactor : https://github.com/urllib3/urllib3/issues/1553 . I didn't verify yet, it's unreleased and how urllib3 client behaves before and after patch for the CRLF characters and similar tests that use urllib3 as client.
Applying the URL encoding patch to cherrypy I can verify that the behavior is also tested
diff --git a/cherrypy/test/test_static.py b/cherrypy/test/test_static.py
index cdd821ae..85a51cf3 100644
--- a/cherrypy/test/test_static.py
+++ b/cherrypy/test/test_static.py
@@ -398,7 +398,8 @@ class StaticTest(helper.CPWebCase):
self.assertInBody("I couldn't find that thing")
def test_null_bytes(self):
- self.getPage('/static/\x00')
+ from urllib.parse import quote
+ self.getPage(quote('/static/\x00'))
self.assertStatus('404 Not Found')
@classmethod
After patch the test passes and ValueError is also raised properly as the null byte is decoded in the server and using it in os.stat to resolve null byte path.
Breakages were expected in the discussion as adopting the fix from golang. Giving an option like a parameter to bypass the validation was also discussed in the linked but giving an option would also mean it could be abused or missed.
----------
_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue36274>
_______________________________________
More information about the Python-bugs-list
mailing list