1196740 - SRI ignores cross-origin restriction when 30x redirect is used for loading target resources

Status: VERIFIED FIXED

(Core :: DOM: Security, defect)

Description

[Muneaki Nishimura]

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.155 Safari/537.36

Steps to reproduce:

The W3C specification (below) of Subresouce Integrity requires UA to verify CORS headers while loading of cross-origin resources because there is a risk of cross-origin  data leakage.
http://www.w3.org/TR/SRI/#cross-origin-data-leakage

But the latest Firefox Nightly 43 ignores CORS headers if 30x redirector on the same origin is used for retrieving cross-origin resources.

Please see the following live demo.
http://mallory.csrf.jp/sri/


Actual results:

At line 6, a cross-origin JS file is loaded regardless of SRI, it's valid behavior.
<script src="http://alice.csrf.jp/sri/credential.js" integrity="sha256-fake" onload ="expected()" onerror="unexpected()"></script>

Howeverm at line 9, the same file is loaded through a 308 redirector on the same origin as HTML, then loading file is blocked by SRI. This behavior violates the W3C SRI specification.
<script src="redirector.php?u=http://alice.csrf.jp/sri/credential.js" integrity="sha256-fake" onload ="expected()" onerror="unexpected()"></script>



Expected results:

SRI should check CORS headers even when 30x redirector is used for loading cross-origin resources.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I this a regression?

Comment 2

[Muneaki Nishimura]

It's not a regression. SRI was newly introduced on Nightly last week by Bug 992096 and it's still disabled by default. You need to set security.sri.enable=true in about:config for testing my PoC (http://mallory.csrf.jp/sri/) at the moment.

Comment 3

[François Marier [:francois]]

Attachment: Fix and tests

The fix is simple and is pretty much what Christoph suggested on IRC.

Stylesheets were unaffected but I've added a test to ensure that this continues to work.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8651333 [details] [diff] [review]
Fix and tests

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

NS_GetFinalChannelURI() to the rescue :-) Since you are passing 'aChannel' as an argument to IsEligible() you can now remove 'aRequestURI' from the argument list, because you are only using it for internal logging. In fact we should also remove it from the argument list of VerifyIntegrity() and just rely on the channel argument. In case you want to use the orginal uri for logging purposes, you can use:
> nsIChannel.idl
>
> attribute nsIURI originalURI;

Looks great otherwise, thanks for adding more redirect tests.

::: dom/security/test/sri/mochitest.ini
@@ +39,1 @@
>  [test_style_sameorigin.html]

nit: remove the empty lines between the [test_*] listings.

Comment 5

[François Marier [:francois]]

Attachment: Fix and tests

Good point about using GetOriginalURI() to get to the request URI. Here's a patch that does this.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8652081 [details] [diff] [review]
Fix and tests

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

Maybe at some day I can convince you to use an *.sjs file to serve headers rather than adding so many ^headers^ files :-)
Thanks for fixing - looks great!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/793bc0200220

Comment 8

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue using the following build with the poc from comment #0:
- https://archive.mozilla.org/pub/firefox/nightly/2015-08-25-03-02-12-mozilla-central/

Checked the browser console and received the following error when the file was loaded through a 308 redirector:

> The hash contained in the integrity attribute has the wrong length.
> None of the "sha256" hashes in the integrity attribute match the content of the subresource.
> NOT EXPECTED!!

Went through verification using the following build:
- https://archive.mozilla.org/pub/firefox/nightly/2015-09-08-03-02-03-mozilla-central/

> CREDENTIAL
> EXPECTED
> "http://mallory.csrf.jp/sri/redirector.php?u=http://alice.csrf.jp/sri/credential.js" is not
> eligible for integrity checks since it's neither CORS-enabled nor same-origin.

OS's Used:

- Win 10 x64 (VM) -> PASSED
- Win 7 x64 (VM) -> PASSED
- OSX 10.10.5 x64 -> PASSED
- Ubuntu 14.04.3 (VM) -> PASSED