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
184 stars 90 forks source link

Host header is not validated #646

Open kasium opened 7 months ago

kasium commented 7 months ago

I'm submitting a ...

🐞 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

webknjaz commented 6 months ago

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

webknjaz commented 6 months ago

@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 commented 6 months ago

@webknjaz this does not show the issue for me

webknjaz commented 6 months ago

@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 commented 6 months ago

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 commented 5 months ago

@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 commented 5 months ago

Sure, let me have a look later this week

kasium commented 5 months ago

@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 commented 2 months ago

Just wondering if this would help? https://github.com/cherrypy/cheroot/pull/722