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

Proposal: Disallow cross-origin iframes usage of window.{alert, confirm, prompt} #5407

Closed
carlosjoan91 opened this issue Mar 26, 2020 · 24 comments · Fixed by #6401
Closed

Proposal: Disallow cross-origin iframes usage of window.{alert, confirm, prompt} #5407

carlosjoan91 opened this issue Mar 26, 2020 · 24 comments · Fixed by #6401
Labels
removal/deprecation Removing or deprecating a feature

Comments

@carlosjoan91
Copy link
Contributor

There have been a few spoofs that use JS dialogs [1][2][3], and in response some browsers have tried to identify the site triggering the alert in the dialog. This however leads to confusing UI in which users are shown a URL that is seen nowhere else, or an attempt at explaining the dialog is not from the top level page nor the browser (e.g. Chrome uses "An embedded page on this page says:..").
Because of this, in Chrome we are considering removing support for cross-origin iframes to trigger JS dialogs (link to Intent to Remove discussion).
The spec allows early returning in all three cases (step 3 of [4][5][6]), but if other browsers are on board with a similar change, cross-origin iframes JS dialogs can be explicitly disallowed by adding an early return step before step 3 for the cross origin iframe case.

Chrome's use counter data says around 0.009% of page loads would be affected with this change (0.006% for alert, 0.003 for confirm and 0.00006 for prompt).

@TimothyGu
Copy link
Member

Here's the link to the Intent to Remove that works outside Google: https://groups.google.com/a/chromium.org/d/msg/blink-dev/hTOXiBj3D6A/JtkdpDd1BAAJ

@annevk
Copy link
Member

annevk commented Mar 27, 2020

Could you clarify the exact origin check in use here? Is it same origin or same origin-domain (or even same site)?

@annevk annevk added the removal/deprecation Removing or deprecating a feature label Mar 27, 2020
@carlosjoan91
Copy link
Contributor Author

The plan is for the check to be same origin. In Chrome that is the boundary we use to decide if we can allow the frame to trigger dialogs that look indistinguishable from top-level triggered ones.

That being said, feedback is welcome if folks think there are good reasons to go with same site or same domain instead.

@annevk
Copy link
Member

annevk commented Mar 31, 2020

A same origin comparison between the environment's origin and top-level origin seems reasonable to me. Pretty sure Firefox would follow if this is successful.

@cdumez thoughts for WebKit?

@domenic
Copy link
Member

domenic commented Mar 31, 2020

Same origin-domain (note: not same domain) would make the most sense to me, since if you're same-origin domain then you could just do top.alert() to get around a strict same-origin restriction.

@carlosjoan91
Copy link
Contributor Author

Yeah, that's a good point, using origin-domain makes sense to match the boundary for top.alert().

@annevk
Copy link
Member

annevk commented Dec 8, 2020

@manoharreddyporeddy
Copy link

Any update on this thread. It is in open for 8 months now. Thank you
Came from googlecolab/colabtools#888

@carlosjoan91
Copy link
Contributor Author

We're still planning to do this in Chrome, and since Mozilla is also considering the change, I'll go ahead and put together a pull request while we hear from Webkit.

@cdumez: please let me know if there are any objections or thoughts from Webkit. Thanks.

@cdumez
Copy link

cdumez commented Jan 15, 2021

We're still planning to do this in Chrome, and since Mozilla is also considering the change, I'll go ahead and put together a pull request while we hear from Webkit.

@cdumez: please let me know if there are any objections or thoughts from Webkit. Thanks.

Seems like a good change.

@carlosjoan91
Copy link
Contributor Author

I created a PR for the change, I'll send it for review once I write a WPT.

domenic added a commit to carlosjoan91/html that referenced this issue Jan 27, 2021
@domenic domenic closed this as completed in 7c8fb86 Feb 8, 2021
@cdumez
Copy link

cdumez commented Feb 9, 2021

Hi, I'd like to ask for some clarification with regards to iframes with sandbox="allow-modals". For example:
<iframe sandbox="allow-scripts allow-modals" src="doAlert.html"></iframe>

It appears this no longer allows doAlert.html to call alert(). This can be worked around by adding allow-same-origin to the sandbox directives:
<iframe sandbox="allow-scripts allow-modals allow-same-origin" src="resources/sandboxed-iframe-ALLOWED-modals-iframe.html"></iframe>

Is this as intended?

@domenic
Copy link
Member

domenic commented Feb 9, 2021

@cdumez great catch. IMO that is working as intended, as a natural consequence of this specification and also as part of the general clampdown on alert/confirm/prompt. However, I'm not sure @carlosjoan91 or @annevk were aware of that consequence while we made this change, so let's reopen and get their opinions.

Also, even if we affirm that this is intended, we should add a non-normative note to the spec explaining that allow-modals only works when combined with allow-same-origin, to explain the situation. So we can use this reopened issue to track that.

@domenic domenic reopened this Feb 9, 2021
@carlosjoan91
Copy link
Contributor Author

Good point! Wasn't aware before, but I agree that this seems to be working as intended after this change.

If there are no objections I can go ahead and add the note to allow-modals.

@annevk
Copy link
Member

annevk commented Feb 10, 2021

Sounds good to me, FWIW.

imhele added a commit to imhele/html that referenced this issue Feb 18, 2021
* Editorial: remove redundant "the"

* Meta: default branch rename

Also correct a broken link. Not even w3.org URLs are that cool.

Helps with whatwg/meta#174.

* Editorial: clean up calls to "parse a URL"

It actually takes a string, so calls should be clear about that.

* Review Draft Publication: January 2021

* Simplify <link>s

In particular, remove their activation behavior, stop them from matching
:link and :visited, and stop suggesting that they be focusable areas.

This also includes a slight expansion and rearrangement of the link
element's section to make it clearer what hyperlinks created by <link>
are meant for, contrasting them to <a> and <area> hyperlinks.

Closes whatwg#4831. Closes whatwg#2617. Helps with whatwg#5490.

* Meta: remove demos/offline/* (whatwg#6307)

These are no longer needed as of e4330d5.

* Meta: minor references cleanup

Use more HTTPS and drop obsolete HTML Differences reference.

* Editorial: anticlockwise → counterclockwise

We use en-US these days. Spotted in https://twitter.com/iso2022jp/status/1352601086519955456.

* Use :focus-visible in the UA stylesheet

See w3c/csswg-drafts#4278.

* Editorial: align with WebIDL and Infra

* Fix "update a style block" early return

The new version matches implementation reality and CSSWG resolution.

The algorithm was also inconsistent, as it looked at whether
the element was in a shadow tree or in the document tree, but it was
only specified to be re-run if the element becomes connected or
disconnected.

The CSSWG discussed this in
w3c/csswg-drafts#3096 (comment)
and http://wpt.live/shadow-dom/ShadowRoot-interface.html tests this.

This also matches closer the definition of <link rel="stylesheet">,
which does use connectedness (though it uses "browsing-context
connected", which is a bit different):
https://html.spec.whatwg.org/#link-type-stylesheet

* Modernize and refactor simple dialogs

This contains a small bug fix, in that confirm() and prompt() said
"return" in some cases instead of "return false" or "return null" as
appropriate.

Other notable changes, all editorial, are:

* Factoring out repeated "cannot show simple dialogs" steps, which will
  likely expand over time (see e.g. whatwg#6297).
* Separating out and explaining the no-argument overload of alert().
* Passing the document through to the "printing steps", instead of just
  having them talk about "this Window object".

* Meta: add definition markup for MessageEvent

* Remove <marquee> events

They are only supported by one engine (Gecko).

Closes whatwg#2957.

* Clarify when microtasks happen

* Ignore COEP on non-secure contexts

Fixes whatwg#6328.

* Editorial: update URL Standard integration

* Editorial: only invoke response's location URL once

Complements whatwg/fetch#1149.

* Track the incumbent settings and active script in Promise callbacks

Closes whatwg#5213.

* createImageBitmap(): stop clipping sourceRect to source's dimensions

It has been found in whatwg#6306 that this was an oversight at the time of its introduction. Current behavior goes against author expectations and no implementer has opposed the change to "no-clip".

Tests: web-platform-tests/wpt#27040.

Closes whatwg#6306.

* Remove CSP plugin-types blocking

With Flash not being supported anymore, the CSP directive plugin-types has lost its main reason for being and is being removed from the Content Security Policy specification: w3c/webappsec-csp#456.

This change removes references to the relevant algorithm from the Content Security Policy spec.

* Meta: set more dfn types

A follow-up to:

* whatwg#5694
* whatwg#5916

* Editorial: occuring → occurring

* Make all plugin-related APIs no-ops

Part of whatwg#6003.

* Disallow simple dialogs from different-origin domain iframes

Closes whatwg#5407.

* Revive @@iterator for PluginArray/MimeTypeArray/Plugin

@@iterator is implicitly installed by defining an indexed property getter. Since there is no other way to define it exclusively, this restores some methods back to being indexed getters.

This fixes an inadvertent observable behavior change in d4f07b8.

* Adjust web+ scheme security considerations to account for FTP removal

Also, network scheme is now reduced to HTTP(S) scheme.

Helps with whatwg#5375, but form submission issue remains.

See whatwg/fetch#1166 for context.

* Meta: export pause

Nobody but XMLHttpRequest take a dependency on this please. You have been warned.

Context: whatwg/xhr#311.

* Fix typo: ancestor → accessor

Fixes whatwg#6374.

Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
Co-authored-by: Domenic Denicola <d@domenic.me>
Co-authored-by: Emilio Cobos Álvarez <emilio@crisal.io>
Co-authored-by: Momdo Nakamura <xmomdo@gmail.com>
Co-authored-by: Jake Archibald <jaffathecake@gmail.com>
Co-authored-by: Yutaka Hirano <yhirano@chromium.org>
Co-authored-by: Shu-yu Guo <syg@chromium.org>
Co-authored-by: Kaiido <tristan.fraipont@gmail.com>
Co-authored-by: Antonio Sartori <anton.sartori@gmail.com>
Co-authored-by: Michael[tm] Smith <mike@w3.org>
Co-authored-by: Ikko Ashimine <eltociear@gmail.com>
Co-authored-by: Carlos IL <carlosjoan91@gmail.com>
Co-authored-by: Kagami Sascha Rosylight <saschanaz@outlook.com>
Co-authored-by: Simon Pieters <zcorpan@gmail.com>
domenic added a commit that referenced this issue Feb 19, 2021
domenic added a commit that referenced this issue Feb 23, 2021
@kpeterson85
Copy link

kpeterson85 commented Apr 26, 2021

We just encountered this change in Chrome beta 91.0.4472.19 and it has caused a couple of platforms to have issues. Will there be any controls around allowing dialogs to still be used from cross origin iframes?

One platform that is affected is Salesforce. In the native Salesforce UI they iframe content from different domains they manage for security reasons. In their iframed pages they will present confirm messages before an action can be taken. Now that the confirm is blocked, users are unable to move forward with the actions so different sections are unusable now.

A second platform that was affected is an online form builder we manage. Clients iframe our hosted forms (different domain) into their website. Normally when visitors click the submit button on an iframed form they receive an alert() if there were any field validation messages. But now when we click the submit button nothing happens because the alert() window is never shown.

I had not heard about this upcoming change and luckily use the beta and came across it. Will there be larger communication about this change so platforms can work on fixes before it arrives to the mainstream?

@addamsviz
Copy link

I have also noticed the issue mentioned by kpeterson85 with Adobe Experience Cloud on backend pages where iframes are used to manage Adobe Analytics. This update seems that it will be very impactful to many platforms that use iframes. Can this change be delayed or reverted so that providers will have adequate time to update applications to work for this significant change?

@carlosjoan91
Copy link
Contributor Author

Thanks for reporting the issues. Since the issues seem to be more related with the timing of Chrome's launch rather than the spec change, I'd suggest we move the discussion to Chromium's bug tracker (there is already an open bug for this at: crbug.com/1204081).
If there are comments regarding the spec change, then those do belong here.

@shakti97
Copy link

I agree with @kpeterson85. Our product is also affected by this change. Some of the functionality stops working. Is there any workaround to allow this @carlosjoan91 for now, So we will have adequate time to adapt the change.

@kpeterson85
Copy link

@carlosjoan91 When I mentioned there being controls around allowing cross origin dialogs I was thinking about attributes on the iframe element. If dialogs are going to be disabled by default for cross origin iframes with the new spec then It would be helpful to give the parent page the ability to add something to the iframe indicating they should be allowed.

@carlosjoan91
Copy link
Contributor Author

I can't speak for other browsers, but from Chrome, we would prefer not having an opt-out attribute. The reason we are planning this change is to make the UI easier to understand for users (and less abuse prone) by not having to explain the message is not coming from the site shown in the URL bar (or the browser itself), and having an opt-out means that case still needs to be handled. Also, there is an alternative for use cases as messages can be displayed in the site's UI instead of using a modal dialog.

@colonelpopcorn
Copy link

Hi All,
Appreciate all the work you all are doing to keep the web usable and safe! However, Chrome 92 broke our application and we are now looking at working into the weekend to update our application to accommodate this breaking change. While we're undoubtedly going to be better off for the changes, I'd like to know how you intend to notify regular users and developers of these changes in the future? Is there a status page we can watch to see what will be deprecated or do you expect web developers to follow w3c mailing/browser spec mailing lists? If the latter, do you have any suggestions on filtering out the signal from the noise? I realize this change was in the works for a year plus, but we were only made aware of the change after it broke our application. Please consider how to make developer experience better in the future, and please don't ever forget Hyrum's law.

Thanks,
Jonathan Ling

@taktikweb
Copy link

Following this thread! Chrome 92 broke our application too. Would love to hear about a workaround anytime soon.

@whatwg whatwg locked as off-topic and limited conversation to collaborators Jul 26, 2021
@whatwg whatwg unlocked this conversation Aug 5, 2021
@BLamy
Copy link

BLamy commented Aug 5, 2021

There is no right way to make this the default behavior without breaking a significant amount of the web.

I frame's should have to opt into this behavior by setting a property.

@whatwg whatwg locked as too heated and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
removal/deprecation Removing or deprecating a feature
Development

Successfully merging a pull request may close this issue.