Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider the origin of the document in <iframe src="data:..."> #1753

Closed
mikewest opened this issue Sep 7, 2016 · 16 comments
Closed

Reconsider the origin of the document in <iframe src="data:..."> #1753

mikewest opened this issue Sep 7, 2016 · 16 comments

Comments

@mikewest
Copy link
Member

mikewest commented Sep 7, 2016

https://html.spec.whatwg.org/multipage/browsers.html#origin defines the origin of a Document object "generated from a data: URL found in another Document or in a script" as "The origin of the incumbent settings object when the navigate algorithm was invoked, or, if no script was involved, the origin of the node document of the element that initiated the navigation to that URL."

Currently, Blink, WebKit, and EdgeHTML violate this requirement, instead placing things like <iframe src="data:..."> into a unique opaque origin. See the "willful violation" comment at https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp?rcl=0&l=90, for instance.

For instance, https://output.jsbin.com/wofepos shows that Chrome, Safari, and Opera all reflect null as the origin of a message from a data: frame and deny DOM access, while Firefox reflects https://output.jsbin.com/ (the origin of the embedder) and allows DOM access. Edge 13 (which I'm stuck on because of exciting corporate update policy) reflects data:// (which is weird) and denies DOM access.

This tiny test, of course, is the tip of an iceberg.

@annevk
Copy link
Member

annevk commented Sep 7, 2016

Note that I think Mozilla in general is resigned to the fact that this is better, despite some folks not liking it too much: https://bugzilla.mozilla.org/show_bug.cgi?id=1018872.

@domenic
Copy link
Member

domenic commented Sep 7, 2016

It sounds like we should align the spec with the majority of implementations for this specific problem. However, I remain a bit confused about the issues discussed in whatwg/fetch#381 and what we want to do there. I guess they can be discussed separately?

@annevk
Copy link
Member

annevk commented Sep 7, 2016

Yeah, they are separate.

@foolip
Copy link
Member

foolip commented Sep 7, 2016

@sicking @bzbarsky @smaug----, would it fall of one of you to implement this change in Gecko, and are you OK with it?

@bzbarsky
Copy link
Contributor

bzbarsky commented Sep 7, 2016

I have no plans to touch this stuff in Gecko. I doubt @sicking does either, since he's not currently actively working on Gecko.

I, personally, am still opposed to changing this at all. It makes no sense to me to treat data: and blob:/srcdoc differently here.

@annevk
Copy link
Member

annevk commented Sep 8, 2016

The attack vector is URL injection. Injecting a same-origin blob URL is hard as it would also require you to be able to execute script. You cannot inject srcdoc as a URL. Therefore data URLs are the only URL source that is vulnerable.

@foolip
Copy link
Member

foolip commented Sep 8, 2016

I tested https://output.jsbin.com/wofepos in Edge 14 via BrowserStack and it says "The origin of a data: frame is: data://, and access was denied." (Unsurprising, given that this was the case in Edge 13 too.)

We have a 3/1 split here, and aligning with the majority looks reasonable. But in terms of reaching interop, we can only claim success if Gecko actually changes, so @annevk, can you figure out who would actually make this change in Gecko?

@mikewest, can you open a web-platform-tests to test both the origin (failing on data://) and that access is denied? Writing the test itself is even better of course :) If you can then send it along to someone on the Edge team there's a good chance they'll fix it.

@annevk
Copy link
Member

annevk commented Sep 8, 2016

@foolip I don't think Gecko has the resources to change this anytime soon, especially given that @bzbarsky is not interested. Perhaps @overholt knows something I don't.

mikewest added a commit to web-platform-tests/wpt that referenced this issue Sep 8, 2016
This patch adds the small test created for whatwg/html#1753, which asserts that
'data:' URL documents have an origin which serializes to 'null', and block DOM
access to excitingly cross-origin properties.
@mikewest
Copy link
Member Author

mikewest commented Sep 8, 2016

FWIW, Gecko's approach is secure in itself. The data: URL's origin is inherited from the context triggering the navigation, and is safely cross-origin from the document into which it's been embedded. The vulnerabilities I've seen related to data: URLs almost universally relate to developer confusion caused by every other browser doing something different. That is, the developer will test in Chrome or Safari (or Opera (or Edge)), see the opaque origin, and say "Great! I can totally reflect user-controlled data in this spot without worrying about anything other than escaping!" Boom, Firefox-only XSS. I know this has happened to both Facebook and Twitter in the past, and would not be surprised it if happens again to other developers in the future.

If everyone had picked up this model a few years ago when it was added to the spec, this wouldn't be a problem. Not everyone did, however, and engines diverged. Regardless of whether this underlying divergence is principled or historical accident, it's real, and it causes real problems for developers. As frustrating as it will be for Gecko to change its behavior here, that seems more likely to be a path to interop than teaching Blink, WebKit, and EdgeHTML how to track origins for cross-frame navigation.

@mikewest
Copy link
Member Author

mikewest commented Sep 8, 2016

Perhaps @hillbrad or @ptoomey3 could weigh in from a developer's perspective?

@hillbrad
Copy link

hillbrad commented Sep 8, 2016

Nobody can depend on data: having same-origin semantics with its parent frame because it only works on Gecko. On the other hand, I have personally used this at Facebook to demonstrate XSS evasions of HTML sanitizers that worked only on Firefox because of this behavior.

If you want to play around, I built this thing awhile back to play with data/blob URIs in Workers and with sandboxing, it may be informative here as well to quickly show how different browsers handle such things in multiply-embedded cases.

http://webappsec-test.info/~bhill2/sandbox/matrix.html

@smaug----
Copy link

Pages have tons of browser specific code, so many sites may rely on same-origin semantic (and certainly many FF addons and tons of tests we have for Gecko relies on same-origin)

@domenic
Copy link
Member

domenic commented Sep 8, 2016

Another item for Gecko compatibility mode perhaps...

@foolip
Copy link
Member

foolip commented Sep 9, 2016

@smaug----, or someone who can speak with some authority for Gecko's DOM team, if we make this spec change, would you like to eventually attempt aligning with it, or would you prefer that we enshrine this lack of interop in the spec using the navigator compatibility mode? Since this isn't merely for feature detection like taintEnabled, I'd prefer to require one behavior so that web-platform-tests keeps telling us that there's a problem here.

@ptoomey3
Copy link

ptoomey3 commented Sep 9, 2016

Nobody can depend on data: having same-origin semantics with its parent frame because it only works on Gecko. On the other hand, I have personally used this at Facebook to demonstrate XSS evasions of HTML sanitizers that worked only on Firefox because of this behavior.

Just came to say "me too"; I've only seen this quirk used to demonstrate Firefox specific XSS vectors.

@foolip
Copy link
Member

foolip commented Sep 13, 2016

OK, so I will go ahead and LGTM/merge #1756 and web-platform-tests/wpt#3663

It's still uncertain how things will work out for Gecko, but if making the change is Gecko is deemed too risky or is attempted and fails, then please check back here so we can look for some other way to reach interop.

foolip pushed a commit that referenced this issue Sep 13, 2016
This patch changes the handling of 'data:' URLs to which user agents
navigate. Rather than inheriting the origin of the settings object
responsible for the navigation, they will be treated as unique,
opaque origins.

This aligns the spec with the behavior found in Chrome, Safari,
Opera, and Edge.

Closes #1753.
foolip pushed a commit to web-platform-tests/wpt that referenced this issue Sep 13, 2016
This patch adds the small test created for whatwg/html#1753, which asserts that
'data:' URL documents have an origin which serializes to 'null', and block DOM
access to excitingly cross-origin properties.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
This patch changes the handling of 'data:' URLs to which user agents
navigate. Rather than inheriting the origin of the settings object
responsible for the navigation, they will be treated as unique,
opaque origins.

This aligns the spec with the behavior found in Chrome, Safari,
Opera, and Edge.

Closes whatwg#1753.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants