Incorrect chunk size parsing behavior

[kenballus]

uhttpd has some overly-permissive behavior when parsing HTTP chunk sizes. Due to the minor risk of request smuggling attacks when uhttpd is deployed behind a reverse proxy with corresponding parsing issues, these should probably be fixed.

The RFC specifies that chunked sizes must consist of one or more hex digits. Further, it specifies that the final chunk must consist of one or more 0s. uhttpd doesn't enforce either of these rules.

For example, the following requests are erroneously accepted by uhttpd:

GET / HTTP/1.1\r\n
Host: whatever\r\n
Transfer-Encoding: chunked\r\n
\r\n
0_0\r\n
\r\n

GET / HTTP/1.1\r\n
Host: whatever\r\n
Transfer-Encoding: chunked\r\n
\r\n
\r0\r\n
\r\n

GET / HTTP/1.1\r\n
Host: whatever\r\n
Transfer-Encoding: chunked\r\n
\r\n
0x1\r\n
Z\r\n
0\r\n
\r\n

[kenballus]

This was tested on uhttpd's master branch (current commit is 34a8a74 at the time of writing)

[jow-]

The above examples do not appear to be correct HTTP requests? The headers are not terminated before the body chunks so the 0_0\r\n, \r0\r\n and 0x1\r\nZ\r\n0\r\n parts are interpreted as broken header lines respectively.

Apart from that, the parsing of the actual chunk size values is indeed too lenient, as we're using strtoul() here which accepts more than we want here (such as initial white space or a 0x prefix)

[kenballus]

Sorry! This was due to a typo in my transcription of the requests. I had forgotten to add the second CRLF following the header block. The report has now been edited.

[jow-]

See proposed change in #4. I am unsure if this error handling is sufficient, if we detect an invalid size at this point, we might've already sent a reply so we can't return with a HTTP 400 Bad Request message. All we can do is abort the parsing and closing the connection.

[stokito]

I can't understand how this can be used by an attacker. Why to fix this?

[kenballus]

I can't understand how this can be used by an attacker. Why to fix this?

This can be used by an attacker for request smuggling when a proxy server in front of uhttpd interprets chunk sizes as their longest valid prefix, but forwards them as-is.

Such a proxy would interpret 0x10 as equivalent to 0, but uhttpd would interpret it as equivalent to 10.

[stokito]

@kenballus thank you. It would be good to check BusyBox httpd because it has a simple parsing of headers but also can act as a forward proxy. Once I get a time I may try to do that, and also check the valyala/fasthttp

[kenballus]

I can very easily check FastHTTP because it's integrated in our HTTP parser cross-tester. Busybox httpd I looked at a while ago, but never got around to testing. I'll make a reminder to check that at some point.

[stokito]

The FastHTTP also has a separate reverse proxy.
I expect that the BB httpd may have a problem but for other webservers that may be less risks. Basically there are much more webservers

Thank you for the http-garden!