Re: Bare CR in header fields

On Thu, 21 Jan 2021 at 04:05, Willy Tarreau <w@1wt.eu> wrote:
>
> Hi Cory,
>
> On Wed, Jan 20, 2021 at 09:06:16PM +0000, Cory Benfield wrote:
> > In particular, I'll note that Roy said, a year and a half ago, "If
> > some browsers interpret CR as a line delimiter in HTTP protocol fields
> > (other than payload content), they have security holes that should be
> > fixed." Is that the position of this working group?
>
> To be honest, that was my immediate thinking when I've read your scary
> story about browsers taking them as field terminators! It indeed means
> that it's likely possible to send them such responses and expect some
> content smuggling over persistent connections:
>
>    HTTP/1.1 200 OK
>    Content-length: 10000
>    X-foo: blah\rTransfer-encoding: chunked
>
>    0\r\n
>    HTTP/1.1 200 OK
>    Content-length: 9900
>
>    This is the content non-compliant clients will take as the response
>    for the second request.
>
> This could for example be used to inject malicious code bypassing client-side
> anti-malware solutions, so it should not be taken lightly.

Fortunately, as noted in the original GitHub thread, I don't think any
browsers are subject to a complete response-smuggling attack. This is
mostly because they appear to enforce different rules around the
termination of the header compared to termination of header fields. I
updated my test to attempt a response smuggling attack and no browser
appeared to fall for the smuggled response. CRCR does not appear to
cause any browser I tested to conclude the header is complete. This is
good news!

However, the parse is still bad. My test example sends:

HTTP/1.1 302 Found<CRLF>
Location: /redirect<CRLF>
Content-Length: 0<CRLF>
x-custom-header: ok<CRLF>
x-custom-header-2: y<CRCR>HTTP/1.1 404 Not Found<CR>Content-Length:
0<CRCR><CRLFCRLF>

Chrome does not get tricked by the 404: it does correctly spot that
the CRLFCRLF is the terminator for the header. However, it mis-parses
the header fields. In the web inspector Chrome clearly notes _two_
"Content-Length: 0" header field lines, and if I add additional header
field lines to the second block those will be parsed as though they
were syntactically correct header field lines.

For my part, I don't think there's a huge security risk here. What I
do think is that we need to be cautious when our specs differ widely
from actual deployment. We've done this in the past with notes in the
RFC that suggest that while the normative requirement is (and per Roy,
always has been) to do X, many implementations do Y. That may be an
option for us here. Otherwise, I'd like to hear from the
currently-mentioned user-agents as to whether they plan to remain
non-conformant in this way.


> > Who is
> > coordinating with these implementations to address that security hole?
>
> That's unclear, as I think that all major implementations were represented
> when this spec was written. However it's certain that it's not always trivial
> for an implementation to spot issues that affect them in so detailed a spec.
> It's also possible that some of these decided to still support that behavior
> for various reasons and to apply special safety belts (e.g. no cache, no
> persistent connections, no cookies etc).
>
> Maybe the simplest would be to open a bug report in each of them's issue
> tracker. Even if they decide to close it, the justfication will remain
> there and accessible to anyone, should new threats pop up.
>
> Just my two cents,
> Willy

Received on Thursday, 21 January 2021 07:50:48 UTC