1467998 - <script> element without "cross-origin" attribute treats service worker synthesized CORS response as opaque

Status: REOPENED

(Core :: DOM: Service Workers, defect, P2)

Description

[1lastBr3ath]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce:

Visit https://raw.cm2.pw/Firefox/




Actual results:

The script error, "ReferenceError: foo is not defined", is written in <textarea>. Though the `script` src is is of same-origin, the request is intercepted and changed by Service Worker which first fetches the content from https://api.ipify.org/?callback=foo&format=jsonp and appends `/* ALTERED */`. 

The content, however, is made readable with CORS policy. If the content is not readable or requested with 'no-cors', a generic "script error" is thrown. Thus, the behavior cannot be considered a security/privacy issue.


Expected results:

Errors should only be available to same-origin requests.

Comment 1

[David Olah]

I managed to reproduce the issue on Firefox 60.0.2, Firefox 61.0b7, and Nightly 62.0a1 (2018-06-12), on Windows 10, Ubuntu 16.04 and Mac OS.

Marking it as new on DOM:Service Workers component.

Comment 2

[1lastBr3ath]

FYI, it works in all modern browsers (Safari, not testd). I'm not sure if it's intentional or if I should report it to all vendors.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

The service worker script is not actually passing a cors response to respondWith().  Its doing this:

  self.addEventListener('fetch', event => {
      let [,url] = event.request.url.match(/fake\.php\?url=(.+)/) || [];
      if(url){
          console.info('Bypassing SOP for: ' + url);
          event.respondWith(modify(unescape(url)));
      }
  });

  const modify = url => {
      return fetch(url, {mode: 'cors', redirect: 'follow'}).then(response=>response.text()).then(text=>{
          if(text)    console.log(text);
          return new Response(text + '/* ALTERED */');
      });
  }

In particular note that its doing:

  return new Response(text + '/* ALTERED */');

This is synthesizing a new Response without any associated URL.  The browser has no practical way to tell if the body that was based to such a Response constructor originated from a CORS cross-origin source.

So this behavior is expected and by design.  An origin is allowed to lie to itself via its own same-origin service worker.  And we allow the service worker to read cross-origin CORS-protected data.

One question, though, if you change your test to return the CORS response from the fetch() directly without altering it, what happens?  I expect you should get muted script errors in firefox in that case.

Comment 4

[Anne (:annevk)]

The whole idea behind <script crossorigin> is that you get non-muted errors for cross-origin scripts. This is all by design.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Well, the <script> element in this case does not have the cross-origin attribute.

Comment 6

[Anne (:annevk)]

Ah right, in that case per the standard it should not be muted. If we do, we might want to fix that. I'll reopen.

Comment 7

[1lastBr3ath]

(In reply to Ben Kelly [:bkelly] from comment #3)

> One question, though, if you change your test to return the CORS response
> from the fetch() directly without altering it, what happens?  I expect you
> should get muted script errors in firefox in that case.

Yeah, if I return cors response from fetch() directly, I get generic "script errors" message.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Ok.  Anne, do you think that is a bug then?  I guess the error muting CORs type check should use the actual response type?

Comment 9

[Anne (:annevk)]

Yeah, at least as far as I can remember we didn't change the standard on the response being authoritative (even though most implementations work differently for the moment). (We did add a restriction that a "same-origin" fetch cannot take a "cors" response as that would result in rather tricky downstream scenarios.)

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Ok, then this is very similar to bug 1467454.  I'll morph this bug into a similar issue for script muting.

The fix should be to adjust this code:

https://searchfox.org/mozilla-central/rev/d544b118e26422877108c42723b26e9bb4539721/dom/script/ScriptLoader.cpp#2101

To look at the LoadInfo tainting value instead of doing a cross-origin principal subsumes check.  Or maybe it could do both if we want a system principal to still be able to get full script errors even for an opaque tainted result.