1589609 - overwrite of response header under few circumstances with new version of mozila browser (misparsed header format), this can disable many security headers

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[rohanbirtia]

Attachment: new browser.png

New version of mozila include other header (like x-content-type-options) into x-frame-options header & it overwrites the x-frame-options header policy and makes it to

Server:Microsoft-IIS/8.5
X-Frame-Options:SAMEORIGIN X-Content-Type-Options: nosniff *note here*
X-Powered-By ASP.NET

resulting no click-jacking protection bypass for same cases.

I have tested with old browsers which differentiate between X-Frame-Options and X-Content-Type-Options.
attached screenshot for both browsers.

Comment 1

[rohanbirtia]

Attachment: old_browser.png

Comment 2

[:Gijs (he/him)]

Please provide a link to a public testcase that sends the headers that you believe are being misparsed.

Comment 3

[rohanbirtia]

Hi team,
I figured out why this happened. Attached a video poc of how it works on that site.

Even if I give the site you wouldn't be able to access because it's an internal application & it'll take time to set up the same server config. but I explained in the video how you can replicate.

Basically, if a response header contains white spaces (any number of ) new mozila doesn't parse it as other headers, it continues from the previous header. Which other browser pareses perfectly and differentiate between other headers. It can be any response header.

As you could see from that internal website screenshot, it can disable many security features.

Comment 4

[rohanbirtia]

Attachment: video.mp4

attached poc for why and how the bug works

Comment 5

[:Gijs (he/him)]

Please do not alter the priority field.

Comment 6

[:Gijs (he/him)]

This looks like it's due to line-folding per rfc7230:

   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.

I don't know what, if anything has changed recently in this space. A bugzilla search doesn't turn up anything. Valentin, maybe you know?

If other browsers really don't do it (which is hard to quickly verify without a public testcase) then perhaps we should follow suit for web compat's sake.

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Attachment: test.http

I was able to reproduce this bug in Chrome too.
Download this file, then run the following command:
while true; do cat test.http | nc -l 8000 -q 0; done then go to http://localhost:8000

I have no idea why the reporter says this doesn't reproduce in Fx56 - I managed to do it with this small test case and with a xpcshell-test.

For what it's worth, the bug can be fixed at this line, by removing the ' ' and '\t' cases.

Also relevant to the conversation: https://lists.w3.org/Archives/Public/ietf-http-wg/2013JanMar/0249.html
I don't know if we have other reports of this issue, but it seems we should fix it.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

PS. I don't exactly see how this would be exploitable, unless a server adds whitespace by mistake in which case it's pretty much their fault.

Comment 9

[rohanbirtia]

Hi valentin,
There seems to cache issue in that application that's why i was not able to reproduce in chrome, sorry about that. Seems like the application team tried to pass the buck to security team.

Thanks.

Comment 10

[Valentin Gosu [:valentin] (he/him)]

So, on a second reading of the rfc I noticed this:

   A user agent that receives an obs-fold in a response message that is
   not within a message/http container MUST replace each received
   obs-fold with one or more SP octets prior to interpreting the field
   value.

So, technically this folding is still allowed - the condition is to replace CRLF 1*( SP / HTAB ) with one or more spaces - which we don't currently do,.
The github fetch polyfill also implements this
So this is probably not a sec bug.

Comment 11

[Junior [inactive]]

Correct me if I'm wrong.

rfc7230 3. Message Format

     HTTP-message   = start-line
                      *( header-field CRLF )
                      CRLF
                      [ message-body ]

where

     start-line     = request-line / status-line
     request-line   = method SP request-target SP HTTP-version CRLF
     status-line = HTTP-version SP status-code SP reason-phrase CRLF

==> start-line always end with CRLF
==> header-filed always next to CRLF

And lets take a look at header-field

     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

Note that obs-fold is only taken into account for field-value, not field-name.
token is defined in RFC 2616 which excludes SP

       token          = 1*<any CHAR except CTLs or separators>
       separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT

In any case, each header line should not start with space.

INVALID, but I can write a test to assert this.

Comment 12

[Junior [inactive]]

Attachment: Bug 1589609 - test for failure in parsing header which start with space, r=valentin

Comment 13

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1589609 - Add unit test for obs-fold header parsing r=JuniorHsu

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0e8a5be4d2c
Add unit test for obs-fold header parsing r=JuniorHsu

Comment 15

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/b0e8a5be4d2c

Comment 16

[Valentin Gosu [:valentin] (he/him)]

Note that the patch didn't change any Firefox behaviour, it just added a test that we correctly implement the obs-fold scenario.

Comment 17

[rohanbirtia]

Hi,
Just for curiosity may I know why this bug id is publically accessible. I still believe it is a security issue as if CRLF injection is possible and the attacker is not able to inject new headers, he can disable security headers.

Comment 18

[Valentin Gosu [:valentin] (he/him)]

(In reply to rohanbirtia from comment #17)

Hi,
Just for curiosity may I know why this bug id is publically accessible. I still believe it is a security issue as if CRLF injection is possible and the attacker is not able to inject new headers, he can disable security headers.

It's not clear to me how an attacker could perform such injection.
Could you explain how that scenario would work? Is it a man in the middle for unsecure HTTP/compromised server with HTTPS/something else?

Comment 19

[rohanbirtia]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #18)

(In reply to rohanbirtia from comment #17)

Hi,
Just for curiosity may I know why this bug id is publically accessible. I still believe it is a security issue as if CRLF injection is possible and the attacker is not able to inject new headers, he can disable security headers.

It's not clear to me how an attacker could perform such injection.
Could you explain how that scenario would work? Is it a man in the middle for unsecure HTTP/compromised server with HTTPS/something else?

Nothing with related HTTP or HTTPS , in some cases like CRLF injection it works like this characters from HTTP request directly reflects into response header. So most of the WAF and application block certain characters like %0a %0d. But although these characters are blocked now using our vulnerability attacker can disable certain headers and escalate.

Want to add here is this example (facebook XSS using CRLF) [https://habr.com/en/company/pt/blog/247709/], If the Researcher would have none this vulnerability instead of creating new header named X-XSS-Protection:0 he could have just overwritten the header.

from the same researcher and my friend bobrov https://hackerone.com/reports/66386 , As you can see to escalate CRLF he used https://www.example.com/xxcrlftest%0aSet-Cookie:test=test3;domain=.example.com, with our vulnerability he could have chained to more severe attack just by adding whitespace.

In our daily testing we face a lot of situation like this where only certain characters were reflected in response, this would be added advantage for attacker.