At first glance, this looks like a spec issue to me; I don't see anything in https://fetch.spec.whatwg.org/#concept-request-redirect-mode that implies something different should happen in the case of |redirect: "error"| for a no-cors request.

horo: could you please help triage? Thanks!

Sorry I meant to be Impact stable rather than RBS

elawrence@ is right, I think. "Main fetch" has a step to taint a response, but it happens only when the response obtained by the step 5 is not a network error. "Scheme fetch" returns a network error returned by "HTTP fetch".

Looks SRI has the same issue though SRI is specified to run "match" on a tainted response which may just always fail.

That said, it looks it's been already discussed. horo@ should be familiar with the details.
https://github.com/whatwg/fetch/issues/79

I think this is a spec issue.
The same problem exists in all other major browsers (Safari, FireFox and Edge).

I think https://github.com/whatwg/fetch/issues/79 is different from this issue.
That fetch spec  issue 79  was to protect a web site which is using service worker and redirection from XSS attack.

But this issue is not related to service worker.
For example you can detect whether the web site visitor is an Gmail user or not using this code:
  fetch("https://mail.google.com/mail/u/0/", {
    mode: 'no-cors',
    redirect: 'error',
    credentials: 'include'
  })
  .then(() => {
    console.log('GMail user');
  }).catch(() => {
    console.log('Not GMail user');
  });


annevk@
What do you think?

It seems this was introduced in https://github.com/whatwg/fetch/issues/66#issuecomment-118638144.

My analysis there seems correct, but the fix didn't restrict things to same-origin requests unfortunately. (We were much worse about enforcing review for pull requests back then unfortunately, although those are still hard to come by. Getting more security on all things service workers touched would be good.)

I think the correct fix here would be enforcing that request's mode is "navigate", "websocket", or "same-origin". That should be an assert somewhere in the fetching and algorithm. If request's mode is something else a TypeError seems appropriate in the API (the Request constructor).

Does that help? I can work on more detailed proposed changes to be made after this and equivalent bugs against other browsers are opened up if that's desirable.

Did OP file an equivalent bug against Firefox? (And maybe Edge/Safari, they shipped this too I guess even though they don't have service workers?)

ahsanejaz26, please copy me (I'm annevk@annevk.nl) if you can. I'd like to make sure we all end up with the same fix.

I have filed for Firefox and Edge. 

Someone needs to file for Safari

I filed https://bugs.webkit.org/show_bug.cgi?id=180347.

Firefox equivalent is https://bugzilla.mozilla.org/show_bug.cgi?id=1422710

Thank you for filing bugs.
I think checking the request's mode looks good.

In my mind it's unclear what kind of attacks this could be used for and thus what the severity should be. Tentatively assigning low severity, but horo or others, could you please outline the risks involved with this bug?

The risk is that you can more easily determine whether a user has an account with a given site (assuming the site offers credential-based redirects, which is quite common). However, a determined attacker is likely also able to figure this out through timing attacks.

In https://bugzilla.mozilla.org/show_bug.cgi?id=1422710#c5 Ben suggested to instead return a network error by modifying step 5 in https://fetch.spec.whatwg.org/#main-fetch as follows:

* request’s mode is "no-cors" 
  1. If request's redirect mode is "error" return a NetworkError.
  2. Set request’s response tainting to "opaque".
  3. Return the result of performing a scheme fetch using request.

I think that would work and would less likely break existing code that does not expect an exception. I would only suggest to also check (and therefore block) "manual" as otherwise we make timing attacks rather easy.

Reassigning to tyoshino@

Is someone planning on fixing this? I'm planning on updating the specification. Since WebKit already published tests I guess this attack is already publicly known, but it would still be nice not to spread the knowledge further without fixes having landed.

(Removing Owner as tyoshino left.)

I'm a bit confused - Is https://github.com/whatwg/fetch/commit/14858d3e9402285a7ff3b5e47a22896ff3adc95d the fix you mention?

Yeah, I was confused and we already made the change. My bad.

Please be aware that I think the upstream test that landed today does not match what landed in the spec.  I'm working on a PR to update the test and will share it here when its ready.

Please see:

https://github.com/w3c/web-platform-tests/pull/10536

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a62f913109fc1566230f5963bbf69ee65274ebc8

commit a62f913109fc1566230f5963bbf69ee65274ebc8
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Apr 24 02:23:37 2018

[Fetch API] Fix redirect leak on "no-cors" requests

The spec issue is now fixed, and this CL follows the spec change[1].

1: https://github.com/whatwg/fetch/commit/14858d3e9402285a7ff3b5e47a22896ff3adc95d

Bug:  791324 
Change-Id: Ic3e3955f43578b38fc44a5a6b2a1b43d56a2becb
Reviewed-on: https://chromium-review.googlesource.com/1023613
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552964}
[delete] https://crrev.com/79ca910048175c9118606b96a840516ace45f6c7/third_party/WebKit/LayoutTests/external/wpt/fetch/api/redirect/redirect-mode-expected.txt
[delete] https://crrev.com/79ca910048175c9118606b96a840516ace45f6c7/third_party/WebKit/LayoutTests/external/wpt/fetch/api/redirect/redirect-mode-worker-expected.txt
[modify] https://crrev.com/a62f913109fc1566230f5963bbf69ee65274ebc8/third_party/blink/renderer/core/fetch/fetch_manager.cc

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Thanks for the report ahsanejaz26@ - the VRP panel decided to award $500 for this! A member of our finance team will be in touch to arrange for payment. Also, how would you like to be credited in the Chrome release notes?

Hi,
Thanks a lot for the reward! 
Crediting like this would be fine "AhsanEjaz - @AhsanEjazA".

Hi,
One more thing, the bug has been disclosed to Webkit and Mozilla already, after Chromium, as stated in early comments of the thread. The fix for Webkit is there, and they assigned it CVE-2018-4117, for their products. https://nvd.nist.gov/vuln/detail/CVE-2018-4117 . I'm not sure if Mozilla and Webkit are "third parties who are not directly involved in fixing the bug" or not.

ahsanejaz26@ - thanks for hilighting that, but it wont' be a problem for the VRP payment

Issue 834895 has been merged into this issue.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot