whatwg / fetch

Fetch Standard
https://fetch.spec.whatwg.org/
Other
2.11k stars 331 forks source link

fetch() "no-cors": cross-origin to same-origin redirect taints response #737

Open annevk opened 6 years ago

annevk commented 6 years ago

Although per step 5 of main fetch https://fetch.spec.whatwg.org/#main-fetch if you're on a same-origin URL again after a redirect the tainting gets reset to "basic", that doesn't appear to happen in implementations.

At least for fetch() they'll taint once you go cross-origin.

I don't think they're consistent in that however as cross-origin to same-origin for images will allow image data extraction.

I guess this requires more research.

domfarolino commented 6 years ago

Interesting; Chrome is indeed one of the violators of not resetting tainting back to "basic" after a fetch() lands on same-origin after cross-origin redirect, but that's a simple fix. Are you considering a spec change here depending on what implementations are doing?

annevk commented 6 years ago

I was initially, but I'm wondering if what browsers do here is preferable, since even though you can read such responses as the result of navigation (<iframe>) or image loads, that doesn't allow you to read the bytes of all such responses (e.g., a PDF).

So I think we probably want to keep this restriction intact where we can (and break it for navigation, images, scripts?, ?).

wanderview commented 6 years ago

I think we should avoid reseting back to basic tainting where we can. This is consistent with how cors requests can never become same-origin again once they cross an origin boundary. It would be a bit unexpected to me to make no-cors less restrictive.

annevk commented 6 years ago

I think I added the reset because of <img> and the way <iframe> works (which is "navigate", not "no-cors"). And because I was sloppy I didn't test fetch() I guess. I'm not entirely sure yet how to fix this, but I agree with you on the desirable outcome.

domfarolino commented 6 years ago

(Whoops, this slipped through the cracks in my email). I agree with this as well here, and think we should avoid resetting back to basic tainting.

youennf commented 6 years ago

+1 to not resetting tainting. I haven't tested it on WebKit but if we are resetting tainting in some cases like canvas, this is probably a bug.

yutakahirano commented 6 years ago

Hi, I'm going to add a test because I was about to change the behavior without noticing this issue. I don't have a strong opinion whether we should reset tainting, but I strongly want to have a consistent behavior across fetch() and other initiators.

yutakahirano commented 6 years ago

The test is here. https://github.com/web-platform-tests/wpt/pull/12566

annevk commented 5 years ago

I created a fix for this for Fetch that I think we should land unless it has a bug I didn't see: #834.

However, A -> B -> A is considered same-origin for <img>, <script>, and similar such contexts and we'll need to make sure that's defined properly there (by having HTML poke into the opaque response) and tested.

I'll leave this issue open until that's fully taken care of.

Hope that seems reasonable to everyone.

youennf commented 5 years ago

In WebKit, there is no specific handling for img or script with that regards. I would believe A->B->A would end up being considered cross origin. I'll check that further.

annevk commented 5 years ago

That sounds exciting, though for <img> a workaround would likely be to use <iframe> which I'm 99% sure on only considers the final response for origin comparisons.

youennf commented 5 years ago

Uploaded a test in https://github.com/web-platform-tests/wpt/pull/14112 According that test, Firefox and Chrome do treat A->B->A image as same origin while Safari does not. Not sure about Edge. It might be worth authoring a similar test for scripts and stylesheets.

That sounds exciting, though for <img> a workaround would likely be to use <iframe> which I'm 99% sure on only considers the final response for origin comparisons.

You are probably right, we probably only consider final URLs for iframes origin checks.

domfarolino commented 5 years ago

Was looking at https://html.spec.whatwg.org/multipage/canvas.html#drawing-images:the-image-argument-is-not-origin-clean which sets a CanvasRenderingContext2D's origin-clean flag to false if image is not origin-clean; it seems like the only criteria considered when determining whether or not an image is not origin-clean is the image's origin with respect to that of the entry settings object.

I think that the origin of an image that undergoes A->B->A redirects will be same-origin with that of the entry settings object, even though the response is CORS-cross-origin. It seems if we want the behavior that @youennf's test proposes, we'd have to make the definition of is not origin-clean also consider whether image's origin is CORS-cross-origin right, similarly to what we do muted errors? (As opposed to what we thought before, where HTML would have to be updated after #834 to allow reading this data from a canvas if desirable)

annevk commented 5 years ago

Yeah, the current definition is wrong, it also doesn't match what browsers do when document.domain is involved, see https://www.w3.org/Bugs/Public/show_bug.cgi?id=28374. I forgot that either way we're gonna have to change HTML.

yutakahirano commented 5 years ago

I accidentally changed Chromium behavior months ago, and it's aligned with the new spec. I've had no complaints so far, so I'm leaning to just accepting the new behavior without having an experiment for compatibility.

mfalken commented 5 years ago

I added A->B->A tests for stylesheets in https://github.com/web-platform-tests/wpt/pull/14452. It looks like Chrome and Firefox currently fail it, but I'm working on changing Chrome to pass it at https://chromium-review.googlesource.com/c/chromium/src/+/1367331 (will send a Blink Intent to Ship shortly).

karendolan commented 4 years ago

Hi, is there a way to tell if Safari 13 is able to past the test written for this https://github.com/web-platform-tests/wpt/pull/14112/files?

yutakahirano commented 4 years ago

If you have a device running Safari 13 you can visit http://wpt.live/fetch/images/canvas-remote-read-remote-image-redirect.html.

https://wpt.fyi/results/fetch/images/canvas-remote-read-remote-image-redirect.html?label=experimental&label=master&aligned has a result for "Safari 113 preview macOS 10.15".