HTTP parser does not handle obs-fold correctly

[dcepelik]

System Information

• OS: GNU/Linux 5.14.16-arch1-1
• Ruby: ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x86_64-linux]
• Version: 0.7.44
• OpenSSL: irrelevant

Description

The HTTP parser is in violation of RFC7230§3.2.4:

Historically, HTTP header field values could be extended over multiple lines by preceding each extra line with at least one space or horizontal tab (obs-fold). This specification deprecates such line folding except within the message/http media type (Section 8.3.1). A sender MUST NOT generate a message that includes line folding (i.e., that has any field-value that contains a match to the obs-fold rule) unless the message is intended for packaging within the message/http media type.

A server that receives an obs-fold in a request message that is not within a message/http container MUST either reject the message by sending a 400 (Bad Request), preferably with a representation explaining that obsolete line folding is unacceptable, or replace each received obs-fold with one or more SP octets prior to interpreting the field value or forwarding the message downstream.

The parser seems unaware of the existence of this problem, and making a request with obs-fold results in multiple headers seen by Iodine.

Rack App to Reproduce

app = proc { |env| [
  200,
  env.to_h.select { |k,v| k.start_with?('HTTP_') }.map { |k,v| ["foo_" + k, v ] }.to_h,
  ["Hello World\n"]]
}
run app

Testing code

printf "GET / HTTP/1.1\r\nHost: x\r\nX: Y\r\n A: B\r\n\r\n" | nc 127.0.0.1 3000

Expected behavior

The request should either be discarded with HTTP/400, or header folding should be processed correctly, as per the linked RFC.

Actual behavior

The request is accepted and the response is

HTTP/1.1 200 OK
connection:keep-alive
foo_http_version:HTTP/1.1
foo_http_host:x
foo_http_x:Y
foo_http_ a:B
content-length:12
date:Fri, 26 Nov 2021 10:30:45 GMT
last-modified:Fri, 26 Nov 2021 10:30:45 GMT

Hello World

demonstrating that the rest of the value of header X was understood as a header with name ␣A, which is incorrect.

[boazsegev]

Wow, @dcepelik ,

Thanks for pointing this out! It looks important! 🤩🤩🤩🙏🏻🙏🏻🙏🏻

I bet that finding this issue wasn't easy 👏🏻👏🏻👏🏻

I think this issue could have raised some interesting security concerns if iodine was used as a proxy instead of an application server (header smuggling could occur if whitespace removal was performed)...

I'm off to read more about this issue and see what I can do to fix it.

Cheers, Bo.

[dcepelik]

Hey @boazsegev, I have found a couple more issues while doing due diligence on Iodine. Please let me know if I can provide more details.

I think this issue could have raised some interesting security concerns if iodine was used as a proxy instead of an application server (header smuggling could occur if whitespace removal was performed)...

If multiple HTTP-aware components are stacked and at least two disagree about the meaning of the message, problems are bound to happen. I was specifically looking for ways to smuggle requests in and found unrelated issues.

[boazsegev]

Hi @dcepelik ,

When coding iodine I made the (poor?) assumption that it would always "hide" behind nginx, so I omitted some conformity tests from the parser (assuming these would be handled before the request arrives).

However, you are right in your approach and I am working on a fix for these issues. Since the parser is written in C, I might also consider replacing it with an open source (MIT licensed) alternative, especially if it is fast and keeps the same behavior (i.e., converts header names to lower case, allow UTF-8 header names, etc').

Very nice and creative work on attempting all these attack vectors and variations 👍🏻