Karthikeyan Singaravelan <tir.kar...@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 <rep...@bugs.python.org>
<https://bugs.python.org/issue36274>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to