cherrypy / cheroot

Cheroot is the high-performance, pure-Python HTTP server used by CherryPy. Docs -->
https://cheroot.cherrypy.dev
BSD 3-Clause "New" or "Revised" License
183 stars 90 forks source link

Cheroot incorrectly allows whitespace after header names #714

Open kenballus opened 3 weeks ago

kenballus commented 3 weeks ago

I'm submitting a ...

🐞 Describe the bug. What is the current behavior? Cheroot incorrectly strips whitespace from the ends of header names.

What is the motivation / use case for changing the behavior? This behavior violates the RFCs and is potentially useful for launching request smuggling attacks.

💡 To Reproduce Steps to reproduce the behavior:

  1. Start a Cheroot-based HTTP server that logs all received message bodies.
  2. Send it GET / HTTP/1.1\r\nHost: whatever\r\nContent-Length : 1\r\n\r\nZ
  3. Observe that it logs Z, even though the Content-Length header name is followed by a space.

💡 Expected behavior A 400 response.

📋 Environment

webknjaz commented 3 weeks ago

Hi, are you able to come up with a Cheroot-only reproducer?

By the way, we have Private vulnerability reporting enabled in the CherryPy org projects: https://github.com/cherrypy/cheroot/security/policy / https://github.com/cherrypy/cheroot/security/advisories — it's best to make such reports in restricted areas of GH.

kenballus commented 3 weeks ago

Hi, are you able to come up with a Cheroot-only reproducer?

Yes. I can reproduce this on a fresh build from main with the following steps:

  1. Run this script:
    
    from base64 import b64encode

from cheroot.wsgi import Server, PathInfoDispatcher as WSGIPathInfoDispatcher

RESERVED_HEADERS = ("CONTENT_LENGTH", "CONTENT_TYPE")

def app(environ, start_response) -> list[bytes]: try: body: bytes = environ["wsgi.input"].read() except ValueError: start_response("400 Bad Request", []) return [] response_body: bytes = ( b'{"headers":['

Server(("0.0.0.0", 80), WSGIPathInfoDispatcher({"/": app})).start()

2. Send it an HTTP request with a space after a `Content-Length` header:

GET / HTTP/1.1\r\n Host: whatever\r\n Content-Length : 1\r\n \r\n Z

3. You should see the following response:

HTTP/1.1 200 OK Content-type: application/json Content-Length: 141 Date: Sat, 08 Jun 2024 13:25:25 GMT Server: Cheroot/10.0.2.dev71+g1ff20b18

{"headers":[["SE9TVA==","d2hhdGV2ZXI="],["Q09OVEVOVF9MRU5HVEg=","MQ=="]],"body":"Wg==","version":"SFRUUC8xLjE=","uri":"Lw==","method":"R0VU"}

4. Base64-decode the headers to see that the leading space was removed:
```sh
printf Q09OVEVOVF9MRU5HVEg= | base64 -d | xxd
00000000: 434f 4e54 454e 545f 4c45 4e47 5448       CONTENT_LENGTH
  1. Further, note that the message body is nonempty, indicating that the Content-Length header was interpreted, despite the trailing space.

By the way, we have Private vulnerability reporting enabled in the CherryPy org projects: https://github.com/cherrypy/cheroot/security/policy / https://github.com/cherrypy/cheroot/security/advisories — it's best to make such reports in restricted areas of GH.

Will do for next time.