Closed Bug 1109693 Opened 5 years ago Closed 5 years ago
Image fails with data:image source and cross Origin flag set
7.75 KB, text/html
11.48 KB, patch
|Details | Diff | Splinter Review|
Related to bug 604496? But it's weird to see NS_ERROR_NOT_AVAILABLE rather than a security error.
I guess this is related to https://bugs.webkit.org/show_bug.cgi?id=69732
e.g. This will not display the image, but if you remove the attribute it does: <!DOCTYPE html> <html> <head lang="en"> <meta charset="UTF-8"> <title></title> </head> <body> <img crossorigin="anonymous" src="data:image/png;base64,R0lGODlhCwALANUAAKt5DN/ETtq+fN+/cvHkxNrBjd/MpriHEvrxctO4KfXpMrSNQNzJpffw4Pz59P/uJP79/NCzGtSoL+vZIbePQNi1Yf/bINrUY+vUaNrTdceXHdrKVcWgGTk0Fv/gKv/vOP/4ncmTEAUEAf/4gv/wONS1J0A8HRQRBP/hKtOyJz04Gv/dKZ5sCf/3HP///wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAEAAC4ALAAAAAALAAsAAAZYQJfQNQgNhkiCBCQhIIUVzAhTeTY0GVNG00AKAhfVJSAYOg6ITWeDODiEhRQpcUp8SgUXBKBArf4eCgAQBhwPFhEiERYPHAYUEy2Sky0TFAwLLJqbLAsMQQA7"> </body> </html>
Yeah, this is buggy. We should be allowing data: here. Specifically, https://html.spec.whatwg.org/#potentially-cors-enabled-fetch treats data: and same-origin equivalently.
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → NEW
Component: Canvas: 2D → ImageLib
Ever confirmed: true
And given that spec requirement, why exactly does nsCORSListenerProxy::Init default to not allowing data: URIs?
OK, we have the following places that are currently not passing "true" here: 1) EventSource. Per spec at https://html.spec.whatwg.org/multipage/comms.html#the-eventsource-interface this uses "potentially CORS-enabled fetch" so looks buggy to me. 2) Navigator.sendBeacon. It just delegates to the fetch spec, with mode set to "CORS", so we land in https://fetch.spec.whatwg.org/#concept-fetch which only allows data: if the "same-origin data URL flag" is set. sendBeacon does not set this flag, so disallowing data: here is correct. 3) <script> loader. Per spec at https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script step 14, this does "potentially CORS-enabled fetch", so we're buggy. 4) nsSyncLoader::LoadDocument. No spec, more restrictive here is probably OK. 5) HTMLMediaElement::LoadResource. https://html.spec.whatwg.org/multipage/embedded-content.html#concept-media-load-resource says to do "potentially CORS-enabled fetch". 6) ChannelMediaResource::OpenChannel. I expect that's the same thing as #5 in spec terms. 7) CORS preflights. Disallowing data: here probably makes sense. 8) XSLT loading. No spec here, afaict. 9) Image loader. That's this bug. 10) Font loader. http://dev.w3.org/csswg/css-fonts-3/#font-fetching-requirements says to do "potentially CORS-enabled fetch" 11) CSS loader. https://html.spec.whatwg.org/multipage/semantics.html#concept-link-obtain says to do "potentially CORS-enabled fetch".
This patch fixes items 3, 5, 6, 9, 11 from comment 6, I think. I added tests for 3, 9, and 11, but I'm really not sure how to go about adding tests for the media code (hence the "I think") bit. For the rest of the items, the situation is as follows: Item 1: I filed bug 1156137 on the fact that this can't possibly work with data: URIs in its current state. Items 2, 7: Behavior is already per spec. Items 4, 8: No spec, no change for now. I'm not sure that's right for #8, btw. Item 10: Turns out to not be an issue, because that code is never reached for data: URIs; in the data: case the font loader doesn't install a nsCORSListenerProxy at all. It still seems to me like a bit of a footgun to have our default not match the HTML spec default, though at least it's a footgun in the direction of "not working" instead of "security bug"...
Attachment #8594576 - Flags: review?(jonas)
Comment on attachment 8594576 [details] [diff] [review] More places that use CORS should in fact allow data: URIs Jonas is out, apparently.
Attachment #8594576 - Flags: review?(jonas) → review?(bugs)
Comment on attachment 8594576 [details] [diff] [review] More places that use CORS should in fact allow data: URIs I think CORSListener should have a bit different kind of Init. Something which takes an enum as the second param, and the enum values are clearly indicating whether data: urls are accepted. But such change could happen in a different bug.
Attachment #8594576 - Flags: review?(bugs) → review+
Filed bug 1157451 on making this an enum.
I have no recollection of why data: was disabled by default
You need to log in before you can comment on or make changes to this bug.