Closed Bug 1431095 Opened 6 years ago Closed 6 years ago

"X-Content-Type-Options: nosniff" does not block JSON in <script> context unlike Chrome

Categories

(Core :: DOM: Security, defect, P5)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: x.schindel, Assigned: evilpie)

Details

(Keywords: dev-doc-complete, Whiteboard: [domsecurity-backlog][wptsync upstream])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20180103231032

Steps to reproduce:

Loading a resource with unsuitable content-type in the following way
  <script onerror="alert('error')" type="application/javascript" src="URL_to_resource_containing_json"></script>
  <object onerror="alert('error')" type="application/json"       data="URL_to_resource_containing_json"></object>
where the resource at "URL_to_resource_containing_json" is sent with header "X-Content-Type-Options: nosniff" and Content-Type "appliaction/json"


Actual results:

No error event is triggered.


Expected results:

Loading a resource with unsuitable content-type does not trigger an error event although the header "X-Content-Type-Options: nosniff" is sent with the response.

This offers the possibility of side-channel attacks (i.e. the behavior of a backend in another domain can be derived cross-origin although the content is not accessible due to SOP)
Christoph, could you triage this? Thanks.
Group: firefox-core-security → dom-core-security
Component: Untriaged → DOM: Security
Flags: needinfo?(ckerschb)
Product: Firefox → Core
(In reply to Andrew McCreight [:mccr8] from comment #1)
> Christoph, could you triage this? Thanks.

Anne, I suppose that's true and we should fire and onerror event. What do you think?
Flags: needinfo?(ckerschb) → needinfo?(annevk)
For <script>, yes. I don't quite understand why this fails though given that we pass http://w3c-test.org/fetch/nosniff/script.html. Is the reason a cross-origin URL? If so, we should expand web-platform-tests coverage.
Flags: needinfo?(annevk)
Has nothing to do with same origin or cross origin. I did a quick check using our own mochitests - still fires an event, must be something else.

diff --git a/dom/security/test/general/test_nosniff.html b/dom/security/test/general/test_nosniff.html
--- a/dom/security/test/general/test_nosniff.html
+++ b/dom/security/test/general/test_nosniff.html
@@ -81,17 +81,17 @@ SpecialPowers.pushPrefEnv({set: [["secur
   scriptWrongType.onload = function() {
     ok(false, "script nosniff wrong type should not load");
     checkFinish();
   }
   scriptWrongType.onerror = function() {
     ok(true, "script nosniff wrong type should not load");
     checkFinish();
   }
-  scriptWrongType.src = "file_nosniff_testserver.sjs?scriptWrongType";
+  scriptWrongType.src = "http://example.com/dom/security/test/general/file_nosniff_testserver.sjs?scriptWrongType";
x.schindel, thanks for reporting. Do you have a live testcase for this problem somewhere? Would be curious to find out what the actual problem is.
Flags: needinfo?(x.schindel)
Description above was not precise enough for <object>.

- <script onerror="alert('error')" type="application/javascript" src="URL_to_resource_containing_json"></script>
  --> no error event triggered, see https://jsfiddle.net/utm6rppa/ (Chrome triggers error event -> OK)

- <object onerror="alert('error')" type="application/json" data="URL_to_resource_containing_json"></object>
  --> no error event triggered if return code = 200, but in case of return code = 400, see https://jsfiddle.net/zhk0ga4p/ (Chrome doesn't trigger event in both cases -> OK)
Flags: needinfo?(x.schindel)
The <object> tag difference is a separate bug -- nothing to do with 'nosniff'. Is it wrong to trigger onerror for an error? I thought that was the point of that handler.

comment 0 with the specified content type (assuming "appliaction" is a typo) should satisfy the 'nosniff' and is expected to load. In the fiddle the loaded resource doesn't seem to have a content-type header at all, at least as far as I can see in the Firefox and Chrome devtools. Chrome gives an error message for the fiddle that the content type application/json is not executable.

* Where did that come from? I see no Content-Type header, and even if they incorrectly used the type attribute hint (which they shouldn't) that wasn't json either.

* if there's an invisible content-type I'm not seeing, our "nosniff" implementation considers application/json to be a valid script content-type. I believe this is what the MIME sniffing spec says.

(In reply to x.schindel from comment #0)
> This offers the possibility of side-channel attacks (i.e. the behavior of a
> backend in another domain can be derived cross-origin although the content
> is not accessible due to SOP)

The Same-origin Policy explicitly doesn't apply to <script> tags. This is grandfathered in: every page can load <script> from everywhere and the content is essentially readable (through toSource() or the effects of the script itself).

needinfo? from Anne for the "should nosniff apply to application/json" spec question
Group: dom-core-security
Flags: needinfo?(annevk)
Seems like that's the problem. This part of "script" loading is defined at https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-nosniff? (including the question mark) which uses the list of valid MIME types at https://html.spec.whatwg.org/multipage/scripting.html#javascript-mime-type which does not include application/json.

https://mimesniff.spec.whatwg.org/ does not mention JSON at all currently.
Flags: needinfo?(annevk)
(In reply to Daniel Veditz [:dveditz] from comment #7)
> comment 0 with the specified content type (assuming "appliaction" is a typo)
> should satisfy the 'nosniff' and is expected to load. In the fiddle the
> loaded resource doesn't seem to have a content-type header at all, at least
> as far as I can see in the Firefox and Chrome devtools. Chrome gives an
> error message for the fiddle that the content type application/json is not
> executable.
> 
> * Where did that come from? I see no Content-Type header, and even if they
> incorrectly used the type attribute hint (which they shouldn't) that wasn't
> json either.
> 
> * if there's an invisible content-type I'm not seeing, our "nosniff"
> implementation considers application/json to be a valid script content-type.
> I believe this is what the MIME sniffing spec says.
> 

There is a bit a strange behavior on the server side regarding a small typo in the URL used in the object tag in the jsfiddle mentioned above. When using the right URL the content type is sent with the response (https://jsfiddle.net/rf8kyxdx/). Behavior is still the same as described above.

But nevertheless: requesting a resource from cross-origin should always lead to the same behavior visible somehow to the requesting page. Otherwise the page could extract data from that differing behavior.

Example:
- request: ...?parameter=<something that might lead to an error at the server depending of backend data>
- response: either 200 or 400/500 depending on backend data

Such requests are especially possible e.g. with OData (see odata.org) which offers a lot of possibilities to define a filter. If the page containing the <object>-tag is able to detect this difference it can derive information about the backend data.

---------------

> (In reply to x.schindel from comment #0)
> > This offers the possibility of side-channel attacks (i.e. the behavior of a
> > backend in another domain can be derived cross-origin although the content
> > is not accessible due to SOP)
> 
> The Same-origin Policy explicitly doesn't apply to <script> tags. This is
> grandfathered in: every page can load <script> from everywhere and the
> content is essentially readable (through toSource() or the effects of the
> script itself).

I don't think that the source code loaded from another origin with <script> is accessible somehow.
Well, I got confused by the caching behavior ...
Unfortunately the responses from the server are cached. Subsequent responses have return code 304 which does not have a content-type (or at least it is not shown in the dev tools).
To avoid this add an arbitrary parameter to the resource URLs like p=1, p=2, ... to always get the response directly from the server.
And to be more precise: it's the etag/if-none-match behavior which leads to the 304-response
This bug is about one of two possible things

1) our implementation of nosniff treats JSON as a valid Javascript type, though the spec does not

2) as the summary says the feature as a whole is "not handled properly" by not firing events.

I agree that 1) is our behavior, and for now it's intentional. We were worried there would be too much breakage if we blocked JSON since people mostly load JSON files as <script> and not other elements. If this bug is about 1) it's probably WONTFIX, though I'm happy to leave it open while we argue with the standards folks (Hi Anne!).

If the bug report is 2) then it hasn't been demonstrated. If you return a content type "junk/junk" do we fire the error? In general I believe we handle no sniff errors properly.
Flags: needinfo?(x.schindel)
Chrome and Safari block JSON in combination with nosniff, no? I don't see why that would not be web-compatible.
Not sure what's unclear here: requesting a resource with content-type 'application/json' and x-content-type-options 'nosniff' must trigger an error event. The current behavior of not triggering an error event for 'application/json' can be misused by an attacker to extract data cross-origin.

Example: https://jsfiddle.net/utm6rppa/

According to the comment above the issue regarding event triggering in <object> is not related to nosniff. So I think we should put this topic to another bug report (-> https://bugzilla.mozilla.org/show_bug.cgi?id=1434967).
Flags: needinfo?(x.schindel)
(In reply to Anne (:annevk) from comment #13)
> Chrome and Safari block JSON in combination with nosniff, no? I don't see
> why that would not be web-compatible.

The MIME-sniff spec is no help: https://mimesniff.spec.whatwg.org/#sniffing-in-a-script-context

We are perfectly compatible with the Fetch spec referenced in comment 8.

    If destination is script-like and one of the following is true, then return blocked:

    mimeType starts with `audio/`, `image/`, or `video/`.
    mimeType is `text/csv`. 

Chrome and Safari block JSON -- OK, and what else? Do we fix this bit for JSON and then wait for more bugs type by type on anything else Chrome and Safari block? I don't want to change our behavior here by reverse-engineering Chrome. We need input from them and update the spec accordingly first.
Flags: needinfo?(annevk)
Priority: -- → P5
Summary: "X-Content-Type-Options: nosniff" header is not handled properly → "X-Content-Type-Options: nosniff" does not block JSON in <script> context unlike Chrome
Whiteboard: [domsecurity-backlog]
I think the confusion is that you are looking at "2.7. Should response to request be blocked due to its MIME type?" (which applies generally) whereas comment 8 points to "3.3.1. Should response to request be blocked due to nosniff?" (which only applies when X-Content-Type-Options: nosniff is present on the resource) which has

> If destination is script-like and mimeType (ignoring parameters) is not a JavaScript MIME type, then return blocked.

and the definition of JavaScript MIME type at https://html.spec.whatwg.org/multipage/scripting.html#javascript-mime-type does not include any JSON MIME type. (For future readers, the list of JavaScript MIME types might move to the aforementioned MIME Sniffing standard per https://github.com/whatwg/mimesniff/pull/58.)
Flags: needinfo?(annevk)
So to fix this we need to change this call https://searchfox.org/mozilla-central/rev/74b7ffee403c7ffd05b8b476c411cbf11d134eb9/netwerk/protocol/http/nsHttpChannel.cpp#1299. Interestingly nsContentUtils::IsScriptType is not even a proper super set of nsContentUtils::IsJavascriptMIMEType. So I suggest we just disallow JSON in IsScriptType and add it to IsPlainTextType, because that is the only another caller of that function. I would rather not start allowing the deprecated versioned JS mime-types. I don't think we strip those version parameters in ContentType()?
Please let's allow them. Less lists is less code is fewer bugs. Also, if we don't we violate the standard.
Assignee: nobody → evilpies
Comment on attachment 8951721 [details] [diff] [review]
Change Content-Type-Options:nosniff allowed script MIME-types to match the spec

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

I am fine with the changes within ProcessXCTO(), but I am not sure what implications the changes within nsContentUtils might have. Probably smaug is in a better position to evaluate the changes related to IsScriptType(). Why not leave nsContentUtils[.h|.cpp] as is?
Attachment #8951721 - Flags: review?(ckerschb)
Attachment #8951721 - Flags: review?(bugs)
Attachment #8951721 - Flags: review+
You added IsScriptType in bug 471020, it's not used anywhere else.
Comment on attachment 8951721 [details] [diff] [review]
Change Content-Type-Options:nosniff allowed script MIME-types to match the spec

(In reply to Tom Schuster [:evilpie] from comment #21)
> You added IsScriptType in bug 471020, it's not used anywhere else.

Ah, facepalm. You are absolutely right. In that case there is no need for smaug to review that change. I extracted those types from IsPlainTextType() to IsScriptType() and now you are moving them back - makes sense. Thanks for the clarification.
Attachment #8951721 - Flags: review?(bugs)
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38626987c0cf
Change Content-Type-Options: nosniff allowed script MIME types to match the spec. r=ckerschb
Whiteboard: [domsecurity-backlog] → [domsecurity-backlog][wptsync upstream error]
https://hg.mozilla.org/mozilla-central/rev/38626987c0cf
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/9571 for changes under testing/web-platform/tests
Whiteboard: [domsecurity-backlog][wptsync upstream error] → [domsecurity-backlog][wptsync upstream]
Keywords: dev-doc-needed
I'm not 100% sure what to update on the docs[0] about this — it looks to be documented as per spec, and the behavior is now fixed in Firefox. Can you let me know what you think needs updating?

Thanks!

[0] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options
Flags: needinfo?(evilpies)
Yes, we now match the spec. We should probably have a compatibility note about that. And update https://developer.mozilla.org/en-US/Firefox/Releases/60
Flags: needinfo?(evilpies)
Tom wrote a great note about it in the rel notes. Cheers!
You need to log in before you can comment on or make changes to this bug.