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

Host header is not validated #646

Open
1 of 3 tasks
kasium opened this issue Feb 9, 2024 · 9 comments
Open
1 of 3 tasks

Host header is not validated #646

kasium opened this issue Feb 9, 2024 · 9 comments
Labels
bug Something is broken triage

Comments

@kasium
Copy link
Contributor

kasium commented Feb 9, 2024

❓ I'm submitting a ...

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

🐞 Describe the bug. What is the current behavior?
An invalid host header which does not conform idna is just passed to the underlying wsgi application w/o any validation

❓ What is the motivation / use case for changing the behavior?
If the host header contains invalid data, this this data is passed as the HTTP_HOST environment field. It can lead to various issues

πŸ’‘ To Reproduce
Code

from flask import Flask
from cheroot.wsgi import Server

app = Flask(__name__)
server = Server(bind_addr=("localhost", 5001), wsgi_app=app)
server.safe_start()

Now send a request to the server where you set the Host header field to foobar/...

πŸ’‘ Expected behavior
Cheroot declines requests with invalid host header field.

πŸ“‹ Environment

  • Cheroot version: 10.0.0
  • Python version: 3.12.0
  • OS: Linux

πŸ“‹ Additional context
This was already reported to flask pallets/flask#5392

@webknjaz
Copy link
Member

Thanks! It'd be useful to know if h11 has this problem so we'd know if #201 would address this problem.

@webknjaz
Copy link
Member

@kasium would something like this be a smaller repro?

from cheroot.wsgi import Server, PathInfoDispatcher

Server(bind_addr=("localhost", 5001), wsgi_app=PathInfoDispatcher({})).safe_start()

@kasium
Copy link
Contributor Author

kasium commented Feb 13, 2024

@webknjaz this does not show the issue for me

@webknjaz
Copy link
Member

@kasium are you saying that it's Flask that crashes when it sees something it doesn't expect?

It could be useful to get a PR with test cases for valid and invalid values of the header.

@kasium
Copy link
Contributor Author

kasium commented Feb 15, 2024

Yes, flask assumes that the Header was validated before but it's not.
A valid header is something like localhost:1234, an invalid one foobar/..

@webknjaz
Copy link
Member

@kasium would you mind composing a pull request with an acceptance/regression test for this? It'd be useful to merge it in even without an implementation (marked with xfail).

@kasium
Copy link
Contributor Author

kasium commented Mar 18, 2024

Sure, let me have a look later this week

@kasium
Copy link
Contributor Author

kasium commented Mar 19, 2024

@webknjaz to write a test I need to know where a validation would happen. Also the error does only occur if e.g. a wsgi app parses the header later down the stack.

I checked the cheroot code further and I think the best place for such a validation would be HTTPRequest.read_request_headers. To keep compatibility, the server could accept a new flag which turns validation on. If the header is not valid, it could then answer with 400 (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host). What do you think?

@rockystone77
Copy link

Just wondering if this would help? #722

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

3 participants