-
Notifications
You must be signed in to change notification settings - Fork 9
client: perform strict chunk size parsing #4
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
base: master
Are you sure you want to change the base?
Conversation
cf071f4
to
3b723c4
Compare
Consider the case in which the chunk size is just |
Also, chunk-extensions do have a grammar that it may be worth considering enforcing. |
Since we're not consuming any body data when serving file requests, forcibly shut down keep-alive connections after requests indicating either a content-length or a chunked transfer encoding in order to avoid interpreting request body data as subsequent HTTP request. Signed-off-by: Jo-Philipp Wich <[email protected]>
Commit 15346de ("client: Always close connection with request body in case of error") added logic to close keep alive connections on HTTP errors due to unconsumed request body data. However, since the check happens after emitting the standard HTTP headers, uhttpd might incorrectly reply with a `Connection: keep-alive` even if it is going to close the connection. Move the check before the emitting of the response headers in order to ensure that we're sending the correct `Connection: close` line. Fixes: 15346de ("client: Always close connection with request body in case of error") Signed-off-by: Jo-Philipp Wich <[email protected]>
The function `uh_header_error()` will tear down the connection after sending the HTTP reply message, so update the `connection_close` flag accordingly to avoid incorrectly emitting a `Connection: keep-alive` header. Signed-off-by: Jo-Philipp Wich <[email protected]>
3b723c4
to
3c0fcf1
Compare
Introduce infrastructure and logic to perform less lenient parsing of HTTP request headers, chunk size headers and content-length values. We can not rely on `strtoul()` to parse hexadecimal chunk sizes or content length values as it accepts a wider range of inputs than what is allowed by the HTTP spec. Decode the chunk sizes and length values manually and fix skipping chunk extension headers while we're at it. Also ensure that there's no trailing garbage after the size and that we bail out on overflows. Also rework the parsing of request header lines, to reject malformed header lines or illegal header names. Fixes: openwrt#3 Fixes: openwrt#5 Signed-off-by: Jo-Philipp Wich <[email protected]>
3c0fcf1
to
3c8b991
Compare
This line in
This can also be problematic if the proxy allows trailing data after the chunk contents (for example, see this bug in pound). Might be worth incorporating into this change. |
Introduce infrastructure and logic to perform less lenient parsing of HTTP request headers, chunk size headers and content-length values.
We can not rely on
strtoul()
to parse hexadecimal chunk sizes or content length values as it accepts a wider range of inputs than what is allowed by the HTTP spec.Decode the chunk sizes and length values manually and fix skipping chunk extension headers while we're at it. Also ensure that there's no trailing garbage after the size and that we bail out on overflows.
Also rework the parsing of request header lines, to reject malformed header lines or illegal header names.
Fixes: #3
Fixes: #5
The function
uh_header_error()
will tear down the connection after sending the HTTP reply message, so update theconnection_close
flag accordingly to avoid incorrectly emitting aConnection: keep-alive
header.Commit 15346de ("client: Always close connection with request body in case of error") added logic to close keep alive connections on HTTP errors due to unconsumed request body data.
However, since the check happens after emitting the standard HTTP headers, uhttpd might incorrectly reply with a
Connection: keep-alive
even if it is going to close the connection.Move the check before the emitting of the response headers in order to ensure that we're sending the correct
Connection: close
line.Fixes: 15346de ("client: Always close connection with request body in case of error")
Since we're not consuming any body data when serving file requests, forcibly shut down keep-alive connections after requests indicating either a content-length or a chunked transfer encoding in order to avoid interpreting request body data as subsequent HTTP request.