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.

Sign up for GitHub

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

Jump to bottom

uhttpd incorrectly accepts empty header names. #5

Open
kenballus opened this issue May 23, 2024 · 3 comments · May be fixed by #4
Open

uhttpd incorrectly accepts empty header names. #5

kenballus opened this issue May 23, 2024 · 3 comments · May be fixed by #4

Comments

@kenballus
Copy link

uhttpd accepts requests with empty header names. These are invalid according to the RFCs.

For example, uhttpd will accept the following request:

GET / HTTP/1.1\r\n
:\r\n
\r\n
@stokito
Copy link

stokito commented May 24, 2024

IMHO it's not critical and from security perspective nothing bad should happen. According to a Postel Law we should accept this.

@kenballus
Copy link
Author

The potential security problem is that some reverse proxies treat \r\n:\r\n as equivalent to \r\n\r\n (i.e., the end-of-headers sequence), but forward it as-is. This allows for request smuggling when used with a server that allows empty header names, like uhttpd.

Other web servers (e.g. Nginx, Apache) don't allow empty header names, so clients don't produce them, so no compatibility is lost. The RFCs are pretty clear that header names must be nonempty.

@stokito
Copy link

stokito commented May 24, 2024

Thank you for the explanation.

jow- added a commit to jow-/uhttpd that referenced this issue May 24, 2024
@jow-
client: perform stricter HTTP request parsing
3c0fcf1 
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]>

wip http parsing

Signed-off-by: Jo-Philipp Wich <[email protected]>
jow- added a commit to jow-/uhttpd that referenced this issue May 24, 2024
@jow-
client: perform stricter HTTP request parsing
3c8b991 
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]>
@jow- jow- linked a pull request May 24, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

client: perform strict chunk size parsing jow-/uhttpd
2 participants
@stokito @kenballus