tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.77k stars 5.51k forks source link

Tornado not sufficiently validating HTTP method #3415

Open shselbst opened 4 months ago

shselbst commented 4 months ago

Steps to reproduce

  1. Start a basic Tornado HTTP/1.1 server.
  2. Send it a request with an invalid method:
    printf '\x22\x28\x29\x2c\x2f\x3a\x3b\x3c\x3d\x3e\x3f\x40\x5b\x5c\x5d\x7b\x7d\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff / HTTP/1.1\r\nHost: whatever\r\n\r\n' \
        | nc localhost 80
  3. Observe that the server responds 405, indicating a syntactically valid but unsupported method, instead of 400, which would indicate a syntactically invalid message. Further, note that the 405 response does not contain an Allow header, even though RFC 9110 requires one to be present.

Expected behavior

Tornado should reject the request because its method is invalid due to containing forbidden characters. The HTTP RFCs define that only the following characters are permitted within an HTTP method:

"!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA

(where DIGIT stands for ASCII digits (0-9) and ALPHA stands for ASCII letters (a-zA-Z))

All of the characters in the above request's method are disallowed, so the request should be rejected with a 400.

Nearly all other HTTP implementations reject this request with 400, including AIOHTTP, Apache httpd, FastHTTP, Go net/http, Gunicorn, H2O, HAProxy, Hyper, Hypercorn, Jetty, Ktor, Libevent, Lighttpd, Mongoose, Nginx, Node.js, LiteSpeed, Passenger, Puma, ServiceTalk, Tomcat, Twisted, OpenWrt uhttpd, Unicorn, Uvicorn, Waitress, WEBrick, and OpenBSD httpd.

Impact

Because 405 is heuristically cacheable, and different servers may have different interpretations of which bytes are invalid in headers, this behavior may be usable for cache poisoning.

Tornado version

master @ 100d4db2ea3ef76d0ad400c890db9e2c94331296

bdarnell commented 3 months ago

We have an open PR for this: #3338

kenballus commented 3 months ago

My bad! I was having Shawn write up some issues for weird behaviors that our fuzzer found, and forgot about that PR. Sorry :)