1428473 - Support X-Content-Type-Options: nosniff when navigating

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P3)

Description

[secator]

Attachment: 4 requests with identical php-url, content && content-type, just different pathname

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36 OPR/49.0.2725.64

Steps to reproduce:

Request:
GET /nosniff.php/test.html

Server response headers:
X-Content-Type-Options: nosniff
Content-Disposition: inline
Content-Type: garbage



Actual results:

If not have sub part content-type (with `/`)
Or if not valid main content-type (before /)
e.g. has 1 or more [\s;] (Content-Type: text[space]/html)

Then content-type detecting from extension from location.pathname (if has).
e.g 
/nosniff.php/test.html
/nosniff.php/test.xml
/nosniff.php/test.swg


Expected results:

Header `X-Content-Type-Options: nosniff` for check Content-type header, but not for detecting content-type. 
If content-type not valid - ok, Firefox must show download dialog.

Comment 1

[:Gijs (he/him)]

Christoph, looks like you worked on XCTO:nosniff, do you have any idea what's going on here?

Comment 2

[Christoph Kerschbaumer [:ckerschb}]

(In reply to :Gijs from comment #1)
> Christoph, looks like you worked on XCTO:nosniff, do you have any idea
> what's going on here?

XCTO:nosniff only applies to script and style types (fwiw, even images turned out to be incompatible), but not top-level documents. Firefox will prompt the download manager if it can't render the requested document in the browser. Is this not what's happening? XCTO:nosniff should have no influence on making that decision.

Comment 3

[secator]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #2)
> XCTO:nosniff only applies to script and style types (fwiw, even images
> turned out to be incompatible), but not top-level documents. Firefox will
> prompt the download manager if it can't render the requested document in the
> browser. Is this not what's happening? XCTO:nosniff should have no influence
> on making that decision.

Ok, but IMHO it's very strange that I can control content-type through pathname. I found vulnerability in SaaS with this trick, but think Firefox not right (so primarily report to Bugzilla)

Comment 4

[:Gijs (he/him)]

(In reply to secator from comment #3)
> (In reply to Christoph Kerschbaumer [:ckerschb] from comment #2)
> > XCTO:nosniff only applies to script and style types (fwiw, even images
> > turned out to be incompatible), but not top-level documents. Firefox will
> > prompt the download manager if it can't render the requested document in the
> > browser. Is this not what's happening? XCTO:nosniff should have no influence
> > on making that decision.
> 
> Ok, but IMHO it's very strange that I can control content-type through
> pathname.

Reporter: Only if the content-type itself is bogus, right?

:jduell, do you know what the spec says ought to happen here, and/or if we're actually doing something wrong (irrespective of XCTO:nosniff) ?


> I found vulnerability in SaaS with this trick

Did you report that to the SaaS vendor? Is that vulnerability still open in the affected server software? Was the same vulnerability open to abuse from other browsers, and if so which (and did you report it to any other browsers)?

Comment 5

[secator]

> Reporter: Only if the content-type itself is bogus, right?

Yeah

> > I found vulnerability in SaaS with this trick
> Did you report that to the SaaS vendor?

No, I'm waiting for Mozilla decision.

> Is that vulnerability still open in the affected server software? 

Yes.

> Was the same vulnerability open to abuse from other browsers, and if so which (and did you report it to any other
> browsers)?

No. Chromium and IE prompt the download manager.

Comment 6

[secator]

For comparison

Content-type: garbage/garbage - donwload manager
Content-type: garbage - render

Comment 7

[Daniel Veditz [:dveditz]]

Anne: should nosniff apply to top-level documents? seems Chrome and IE are behaving differently from Firefox.

Comment 8

[Anne (:annevk)]

Is there any difference here with using X-Content-Type-Options: nosniff or not in other browsers? Currently the specification doesn't define anything about nosniff for top-level loads, but maybe it should.

Pathname-based sniffing is a thing that exist in the HTML Standard for plugins, but again, not for top-level loads, just <embed> and maybe <object>.

There is content-based sniffing as per https://mimesniff.spec.whatwg.org/, but that shouldn't detect XML.

It'd be good to have some tests.

Comment 9

[secator]

XML detect:
- if pathname ends with .xml 
- without extension but content start with <?xml ... ?>

HTML detect:
- if pathname ends with .html 
- without extension but content contains html tags

Comment 10

[Anne (:annevk)]

I tested 

  HTTP/1.1 200 OK
  Content-Type: garbage
  X-Content-Type-Options: nosniff

  <?xml version="1.0"?><test/>

both with and without the X-Content-Type-Options: nosniff header and both with and without a .xml path end. The .xml in the path did not matter, but the header did.

With the header present Chrome will render as text and Safari will download. Firefox always renders as XML.

Comment 11

[Anne (:annevk)]

I suspect we could fix this by having the same X-Content-Type-Options: nosniff check we have elsewhere here: https://searchfox.org/mozilla-central/source/netwerk/base/nsBaseChannel.cpp#838. Jason can probably weigh in on that.

(Fixing the corresponding standard for this seems harder and probably requires more time, but we don't have to wait on that.)

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to secator from comment #5)
> > > I found vulnerability in SaaS with this trick
> > Did you report that to the SaaS vendor?
> 
> No, I'm waiting for Mozilla decision.

You should report it to the vendor. Even if we do change the behavior here (and there's not much in the nosniff spec) there are lots of old Firefox around that this SaaS vendor should try to protect.

Comment 13

[secator]

> You should report it to the vendor.

I sent a report. Triaged.

Comment 14

[Frederik Braun [:freddy]]

FYI, I just got a twitter message from an external reporter about this very same bug.
Chrome disallows sniffing even if the content-type is empty (or garbage), when a nosniff-header is present.

This is in contradiction with the spec §7.1, where it says that if the "supplied MIME type" is null or undefined, it should sniff. Regardless of the nosniff header. (https://mimesniff.spec.whatwg.org/#determining-the-computed-mime-type-of-a-resource)


Maybe it's time to change the spec and modify the implementation as suggested by annevk in comment 11?

Comment 15

[Anne (:annevk)]

I filed https://github.com/whatwg/mimesniff/issues/82 on changing the standard. Changing the standard is quite a bit of effort that I hope to make time for, but I wouldn't want to block this bug on that. If we can disable sniffing when X-Content-Type-Options: nosniff is present quite easily we should do so.

Comment 17

[Jason Duell]

Dragana: looks like comment 15 is the fast fix here

Comment 18

[Hanno Boeck]

Just FYI, I plan to make a larger disclosure on mime type issues in early February.

I consider the fact that XCTO nosniff does not disable HTML sniffing a security flaw. The spec may say so, but this heavily violates expectations and opens up practical XSS attacks (see e.g. bug 1509518).

Comment 19

[Christoph Kerschbaumer [:ckerschb}]

Attachment: xcto_nosniff.patch

Dragana, as discussed on Slack I am trying to tackle that bug. Here is my approach:

• In case XCTO nosniff is present within ProcessXCTO() then I set a newly added flag on the loadinfo - I call it skipContentSniffing.
• Later in the NsNetutil where we actually do the sniffing, we just bail out early in case the flag is true within the loadinfo.

I am not sure if that is the best approach, you mentioned that we might be able to use the channel flag LOAD_CALL_CONTENT_SNIFFERS - do you think it might be possible to just remove the flag in ProcessXCTO if necessary?

Finally, how do we create an automated test for that?

Comment 20

[Frederik Braun [:freddy]]

Regarding automated tests:

• load file-bogus-content-type-nosniff.html in an iframe
• content of the file should be along the lines of <script>ok(false, 'This shouldnt parse as HTML or execute JS')</script>
• supply headers as in comment 0, via a ^headers file.

If I'm not mistaken we also seem to have tests for the download prompt in /uriloader/exthandler/tests/mochitest/download.sjs#6

Comment 21

[Dragana Damjanovic [:dragana]]

An example of a sniffer test is:

https://searchfox.org/mozilla-central/source/netwerk/test/unit/test_plaintext_sniff.js

you can do something like that.

Comment 22

[Dragana Damjanovic [:dragana]]

Comment on attachment 9048429 [details] [diff] [review]
xcto_nosniff.patch

Review of attachment 9048429 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like LOAD_CALL_CONTENT_SNIFFERS only prevents sniffing by the listeners, like imgloader, mediasniffer (the once that implement GetMIMETypeFromContent[1]) The sniffer is called after [3]

We also have other sniffing like nsUnknownDecoder. It calls NS_SniffContent but it also does other sniffing[2]. Neither using LOAD_CALL_CONTENT_SNIFFERS nor your change in NS_SniffContent will prevent all sniffings in nsUnknownDecoder. We will need to prevent nsUnknownDecoder as well. It would be good to have test to verify this...

[1] https://searchfox.org/mozilla-central/search?q=symbol:_ZN17nsIContentSniffer22GetMIMETypeFromContentEP10nsIRequestPKhjR12nsTSubstringIcE&redirect=false
[2] https://searchfox.org/mozilla-central/source/netwerk/streamconv/converters/nsUnknownDecoder.cpp#472
[3] https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#1648

Comment 23

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Dragana Damjanovic [:dragana] from comment #22)

Comment on attachment 9048429 [details] [diff] [review]
xcto_nosniff.patch

Thanks Dragana for the detailed notes - I'll look into that and also write some automated tests!

Comment 24

[Christoph Kerschbaumer [:ckerschb}]

Basti will take over this bug - assigning to him.

Comment 25

[Christoph Kerschbaumer [:ckerschb}]

Attachment: xcto_nosniff.patch

Basti, here is a half baked patch (including test), both the patch as well as the test need to be modified and updated. I started working on this but then had too many other things on my plate.

In short: We have to identify all the cases where we do sniffing for navigations and bail out of that sniffing. I guess the easiest way is to set a flag as in the patch on the loadinfo and then just check the flag in those cases - we can talk more whenever you start working on it.

Comment 26

[Anne (:annevk)]

Please also compare with Chrome Canary and Safari Technology Preview to ensure we're reasonably compatible. (It's not going to be great as the fallbacks are different, from what I remember, but we should know what we're in for.)

Comment 27

[Sebastian Streich [:sstreich]]

Attachment: Bug 1428473 Support X-Content-Type-Options: nosniff when navigating r=ckerschb

Comment 28

[Sebastian Streich [:sstreich]]

Attachment: Bug 1428473 Support X-Content-Type-Options: nosniff when navigating r=ckerschb

Comment 29

[Sebastian Streich [:sstreich]]

Hey, i already applied the feedback of Henri (hsivonen) on the Patch but as he's currently away (until august) and it would be great to land the patch this cycle.
Is there anyone else from the html/parser team that could possibly review the Patch again?

Thanks So much,
Basti :)

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/0aa6dca8717afcf336ed340caa233841104701c5

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/0aa6dca8717a

Comment 33

[Henri Sivonen (:hsivonen)]

Sorry, I commented on bug 1602213 before realizing that this one was still hidden. (It was easier to notice that bugs were restricted when the whole background color was different instead of just a banner at the top. Please hide bug 1602213 if considered necessary.)

Chromium still runs its encoding detector, so for non-Latin pages, we can end up in a situation where a page is completely unreadable in Firefox but readable in Chromium. See http://dolnocerovene.tk/site/novini.php . This is a Web compat problem.

Do we have a really strong security reason to turn off the encoding detector when Chromium doesn't? I.e. do we have a strong reason not to take the patch I attached to bug 1602213?

Comment 34

[Henri Sivonen (:hsivonen)]

...and that bug got another duplicate. I think it's not practical to let nosniff affect the character encoding.

Comment 35

[Henri Sivonen (:hsivonen)]

Explaining on this hidden side why I posted a patch that undoes a part of this for review:

I think it's not workable for Firefox to let headers that people add to legacy content "for security" break the content in ways that the content doesn't break in in Chrome. The breakage from security headers should be limited to what is easily discovered by testing in Chrome. Otherwise, we create a Web compat problem.

Furthermore, for avoidance of encoding sniffing to be an effective security measure, we'd need a story for why it would be OK to go to the previous state where lack of sniffing could still result in different interpretations on .com based on the UI localization.