HTTP parser handles line terminators incorrectly

[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:

header-field   = field-name ":" OWS field-value OWS

field-name     = token
field-value    = *( field-content / obs-fold )
field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
field-vchar    = VCHAR / obs-text

obs-fold       = CRLF 1*( SP / HTAB )
	       ; obsolete line folding
	       ; see Section 3.2.4

Also, in 3.5. Message Parsing Robustness:

Although the line terminator for the start-line and header fields is the sequence CRLF, a recipient MAY recognize a single LF as a line terminator and ignore any preceding CR.

But it appears the parser is using \r and \n almost interchangeably.

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

First example

printf "GET /xyz HTTP/1.1\r\nHost: foo\r\nContent-length: 0\r\nU:V\rX:Y\r\n\r\n" | nc 127.0.0.1 3000 | hexdump -C

Second example

printf "GET /xyz HTTP/1.1\r\nHost: foo\r\nContent-length: 0\r\n\rGET / HTTP/1.1\r\nHost: foo\r\nContent-length: 0\r\n\r\n" | nc 127.0.0.1 3000 | hexdump -C

Expected behavior

In the first example, the request should be discarded, as \r is not a valid field-vchar as per the RFC.

In the second example, it would be safest to discard the request (and close connection to the client).

Actual behavior

First example

The request is accepted and the output of the command above is:

00000000  48 54 54 50 2f 31 2e 31  20 32 30 30 20 4f 4b 0d  |HTTP/1.1 200 OK.|
00000010  0a 63 6f 6e 6e 65 63 74  69 6f 6e 3a 6b 65 65 70  |.connection:keep|
00000020  2d 61 6c 69 76 65 0d 0a  66 6f 6f 5f 68 74 74 70  |-alive..foo_http|
00000030  5f 76 65 72 73 69 6f 6e  3a 48 54 54 50 2f 31 2e  |_version:HTTP/1.|
00000040  31 0d 0a 66 6f 6f 5f 68  74 74 70 5f 68 6f 73 74  |1..foo_http_host|
00000050  3a 66 6f 6f 0d 0a 66 6f  6f 5f 68 74 74 70 5f 75  |:foo..foo_http_u|
00000060  3a 56 0d 58 3a 59 0d 0a  63 6f 6e 74 65 6e 74 2d  |:V.X:Y..content-|
00000070  6c 65 6e 67 74 68 3a 31  32 0d 0a 64 61 74 65 3a  |length:12..date:|
00000080  46 72 69 2c 20 32 36 20  4e 6f 76 20 32 30 32 31  |Fri, 26 Nov 2021|
00000090  20 31 31 3a 30 35 3a 30  33 20 47 4d 54 0d 0a 6c  | 11:05:03 GMT..l|
000000a0  61 73 74 2d 6d 6f 64 69  66 69 65 64 3a 46 72 69  |ast-modified:Fri|
000000b0  2c 20 32 36 20 4e 6f 76  20 32 30 32 31 20 31 31  |, 26 Nov 2021 11|
000000c0  3a 30 35 3a 30 33 20 47  4d 54 0d 0a 0d 0a 48 65  |:05:03 GMT....He|
000000d0  6c 6c 6f 20 57 6f 72 6c  64 0a                    |llo World.|
000000da

demonstrating that the \r has become part of the value of the header U.

Second example

The \r\n\r sequence is understood as a terminator for the request, resulting in two requests being handled by Iodine:

00000000  48 54 54 50 2f 31 2e 31  20 32 30 30 20 4f 4b 0d  |HTTP/1.1 200 OK.|
00000010  0a 63 6f 6e 6e 65 63 74  69 6f 6e 3a 6b 65 65 70  |.connection:keep|
00000020  2d 61 6c 69 76 65 0d 0a  66 6f 6f 5f 68 74 74 70  |-alive..foo_http|
00000030  5f 76 65 72 73 69 6f 6e  3a 48 54 54 50 2f 31 2e  |_version:HTTP/1.|
00000040  31 0d 0a 66 6f 6f 5f 68  74 74 70 5f 68 6f 73 74  |1..foo_http_host|
00000050  3a 66 6f 6f 0d 0a 63 6f  6e 74 65 6e 74 2d 6c 65  |:foo..content-le|
00000060  6e 67 74 68 3a 31 32 0d  0a 64 61 74 65 3a 46 72  |ngth:12..date:Fr|
00000070  69 2c 20 32 36 20 4e 6f  76 20 32 30 32 31 20 31  |i, 26 Nov 2021 1|
00000080  31 3a 31 31 3a 32 35 20  47 4d 54 0d 0a 6c 61 73  |1:11:25 GMT..las|
00000090  74 2d 6d 6f 64 69 66 69  65 64 3a 46 72 69 2c 20  |t-modified:Fri, |
000000a0  32 36 20 4e 6f 76 20 32  30 32 31 20 31 31 3a 31  |26 Nov 2021 11:1|
000000b0  31 3a 32 35 20 47 4d 54  0d 0a 0d 0a 48 65 6c 6c  |1:25 GMT....Hell|
000000c0  6f 20 57 6f 72 6c 64 0a  48 54 54 50 2f 31 2e 31  |o World.HTTP/1.1|
000000d0  20 32 30 30 20 4f 4b 0d  0a 63 6f 6e 6e 65 63 74  | 200 OK..connect|
000000e0  69 6f 6e 3a 6b 65 65 70  2d 61 6c 69 76 65 0d 0a  |ion:keep-alive..|
000000f0  66 6f 6f 5f 68 74 74 70  5f 76 65 72 73 69 6f 6e  |foo_http_version|
00000100  3a 48 54 54 50 2f 31 2e  31 0d 0a 66 6f 6f 5f 68  |:HTTP/1.1..foo_h|
00000110  74 74 70 5f 68 6f 73 74  3a 66 6f 6f 0d 0a 63 6f  |ttp_host:foo..co|
00000120  6e 74 65 6e 74 2d 6c 65  6e 67 74 68 3a 31 32 0d  |ntent-length:12.|
00000130  0a 64 61 74 65 3a 46 72  69 2c 20 32 36 20 4e 6f  |.date:Fri, 26 No|
00000140  76 20 32 30 32 31 20 31  31 3a 31 31 3a 32 35 20  |v 2021 11:11:25 |
00000150  47 4d 54 0d 0a 6c 61 73  74 2d 6d 6f 64 69 66 69  |GMT..last-modifi|
00000160  65 64 3a 46 72 69 2c 20  32 36 20 4e 6f 76 20 32  |ed:Fri, 26 Nov 2|
00000170  30 32 31 20 31 31 3a 31  31 3a 32 35 20 47 4d 54  |021 11:11:25 GMT|
00000180  0d 0a 0d 0a 48 65 6c 6c  6f 20 57 6f 72 6c 64 0a  |....Hello World.|

[boazsegev]

Hi!

Very nice smuggling opportunity here if you can get the other layer to ignore this part :) 👏🏻👏🏻👏🏻

The second request issue is a no-brainer must fix.

But the first request...? I'm not sure that's a parsing error or a standard error.

HTTP/2 allows for a wider range of HTTP header values. I think an \r in the middle of a value is acceptable when routed through HTTP/2... so if we want this parser to accept headers from an HTTP/2 reverse proxy (i.e., nginx), isn't this the expected behavior?

More importantly does this expose possible attack vectors...?

[dcepelik]

I think

demonstrating that the \r has become part of the value of the header U.

was what bothered me the most.