Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cheroot incorrectly allows whitespace after header names #714

Open
1 of 3 tasks
kenballus opened this issue Jun 7, 2024 · 2 comments
Open
1 of 3 tasks

Cheroot incorrectly allows whitespace after header names #714

kenballus opened this issue Jun 7, 2024 · 2 comments
Labels
bug Something is broken triage

Comments

@kenballus
Copy link

❓ I'm submitting a ...

  • 🐞 bug report
  • 🐣 feature request
  • ❓ question about the decisions made in the repository

🐞 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

  • Cheroot version: 10.0.2.dev71+g1ff20b18
  • Python version: 3.11.9
  • OS: Linux 6.9.1
@webknjaz
Copy link
Member

webknjaz commented Jun 7, 2024

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
Copy link
Author

kenballus commented Jun 8, 2024

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":['
        + b",".join(
            b'["'
            + b64encode(k.encode("latin1")[len("HTTP_") if k not in RESERVED_HEADERS else 0 :])
            + b'","'
            + b64encode(environ[k].encode("latin1"))
            + b'"]'
            for k in environ
            if k.startswith("HTTP_") or k in RESERVED_HEADERS
        )
        + b'],"body":"'
        + b64encode(body)
        + b'","version":"'
        + b64encode(environ["SERVER_PROTOCOL"].encode("latin1"))
        + b'","uri":"'
        + b64encode(
            (
                environ["PATH_INFO"] + (("?" + environ["QUERY_STRING"]) if environ["QUERY_STRING"] else "")
            ).encode("latin1")
        )
        + b'","method":"'
        + b64encode(environ["REQUEST_METHOD"].encode("latin1"))
        + b'"}'
    )
    start_response(
        "200 OK", [("Content-type", "application/json"), ("Content-Length", f"{len(response_body)}")]
    )
    return [response_body]

Server(("0.0.0.0", 80), WSGIPathInfoDispatcher({"/": app})).start()
  1. 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
  1. 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"}
  1. Base64-decode the headers to see that the leading space was removed:
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken triage
Projects
None yet
Development

No branches or pull requests

2 participants