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

\xa0 and \x85 are stripped from the ends of header values #432

Closed
kenballus opened this issue Feb 5, 2024 · 1 comment · Fixed by #434
Closed

\xa0 and \x85 are stripped from the ends of header values #432

kenballus opened this issue Feb 5, 2024 · 1 comment · Fixed by #434

Comments

@kenballus
Copy link
Contributor

Given that these bytes are allowed in header values (due to obs-text), and are not considered whitespace by the standard, they shouldn't be stripped during header-field OWS stripping.

An example request that demonstrates the bug:

GET / HTTP/1.1\r\n
Test: \xa0\x85abc\x85\xa0\r\n
Host: a\r\n
\r\n

Waitress will see the Test header as having a value of abc, but the vast majority of other HTTP implementations will see it as having a value of \xa0\x85abc\x85\xa0.

@kenballus
Copy link
Contributor Author

I have confirmed that this bug is present in the current main branch of Waitress:

$ git log -n 1
commit 4e0d8c4a951e9a274889609f90fbe31d5253fa82 (HEAD -> main, origin/main, origin/HEAD)
Author: Delta Regeer <[email protected]>
Date:   Sun Feb 4 16:30:32 2024 -0700

    Prep 3.0.0

To verify this for yourself, run the following script:

import base64
from waitress import serve

RESERVED_HEADERS = ("CONTENT_LENGTH", "CONTENT_TYPE")


def app(environ, start_response) -> list[bytes]:
    response_body: bytes = (
        b'{"headers":['
        + b",".join(
            b'["'
            + base64.b64encode(k.encode("latin1")[len("HTTP_") if k not in RESERVED_HEADERS else 0 :])
            + b'","'
            + base64.b64encode(environ[k].encode("latin1"))
            + b'"]'
            for k in environ
            if k.startswith("HTTP_") or k in RESERVED_HEADERS
        )
        + b']}'
    )
    start_response(
        "200 OK", [("Content-type", "application/json"), ("Content-Length", f"{len(response_body)}")]
    )
    return [response_body]


if __name__ == "__main__":
    serve(app, listen="127.0.0.1:8088")

Then send it a request with \xa0 and \x85 on either side of a header value:

printf 'GET / HTTP/1.1\r\nTest: \xa0\x85a\xa0\x85\r\n\r\n' | timeout 1 nc localhost 8088

And observe its output:

HTTP/1.1 200 OK
Content-Length: 33
Content-Type: application/json
Date: Mon, 05 Feb 2024 00:46:59 GMT
Server: waitress

{"headers":[["VEVTVA==","YQ=="]]}

Base64-decoding YQ==, we see that the reported value is a, when it should be \xa0\x85a\xa0\x85.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Feb 5, 2025
3.0.2 (2024-11-16)

Security

- When using Waitress to process trusted proxy headers, Waitress will now
  update the headers to drop any untrusted values, thereby making sure that
  WSGI apps only get trusted and validated values that Waitress itself used to
  update the environ. See Pylons/waitress#452 and
  Pylons/waitress#451


3.0.1 (2024-10-28)

Backward Incompatibilities

- Python 3.8 is no longer supported.
  See Pylons/waitress#445.

Features

- Added support for Python 3.13.
  See Pylons/waitress#445.

Security

- Fix a bug that would lead to Waitress busy looping on select() on a half-open
  socket due to a race condition that existed when creating a new HTTPChannel.
  See Pylons/waitress#435,
  Pylons/waitress#418 and
  GHSA-3f84-rpwh-47g6

  With thanks to Dylan Jay and Dieter Maurer for their extensive debugging and
  helping track this down.

- No longer strip the header values before passing them to the WSGI environ.
  See Pylons/waitress#434 and
  Pylons/waitress#432

- Fix a race condition in Waitress when `channel_request_lookahead` is enabled
  that could lead to HTTP request smuggling.
arnout pushed a commit to buildroot/buildroot that referenced this issue Feb 7, 2025
Both 3.0.1 and 3.0.2 fix security issues.

In 3.0.1:

* Fix a bug that would lead to Waitress busy looping on select() on a
  half-open socket due to a race condition that existed when creating a
  new HTTPChannel. See Pylons/waitress#435,
  Pylons/waitress#418 and
  GHSA-3f84-rpwh-47g6

* With thanks to Dylan Jay and Dieter Maurer for their extensive
  debugging and helping track this down.

* No longer strip the header values before passing them to the WSGI
  environ. See Pylons/waitress#434 and
  Pylons/waitress#432

* Fix a race condition in Waitress when channel_request_lookahead is
  enabled that could lead to HTTP request smuggling.
  See GHSA-9298-4cf8-g4wj

In 3.0.2:

* When using Waitress to process trusted proxy headers, Waitress will
  now update the headers to drop any untrusted values, thereby making sure
  that WSGI apps only get trusted and validated values that Waitress
  itself used to update the environ. See
  Pylons/waitress#452 and
  Pylons/waitress#451

Full Changelog:
https://docs.pylonsproject.org/projects/waitress/en/latest/#change-history

Signed-off-by: Marcus Hoffmann <[email protected]>
Signed-off-by: Peter Korsgaard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant