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.

Sign up for GitHub

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

Jump to bottom

Safari Devtools flooded with security errors on react-dom selection work with iframes with diff origins #14002

Closed
asci opened this issue Oct 27, 2018 · 7 comments
Closed

Safari Devtools flooded with security errors on react-dom selection work with iframes with diff origins #14002

asci opened this issue Oct 27, 2018 · 7 comments
Labels
Browser: Safari Component: DOM Type: Bug

Comments

@asci
Copy link

asci commented Oct 27, 2018

hey folks, looks like this code added small issue with safari and it could flood devtools console output with messages like this:
screenshot 2018-10-26 at 15 25 33
As I understand it happens here and I see you catch the error, but safari still adds the error message if you have iframes with different origin
@philipp-spiess
Copy link
Contributor

@asci Thanks for reporting this issue. Do you have the time to create a reproduction case for us so we can look for potential solutions?

@asci
Copy link
Author

asci commented Oct 29, 2018 •
edited
Loading

@philipp-spiess here it is https://gist.github.com/asci/5f1afb9a772392adf318c624deac9a7a
to reproduce the issue you need to serve there files from different locations and then click on textarea and check console output

one of potential solutions:
to check iframe src origin before accessing contentDocument

while (element instanceof win.HTMLIFrameElement && element.src.indexOf(location.origin) === 0) {

If it's good we can create PR

@asci
Copy link
Author

asci commented Nov 27, 2018

hello! any ideas\updates on this?

@jashkenas
Copy link

jashkenas commented Dec 17, 2018 •
edited
Loading

I was just about to write up a different issue for this — but instead, I'll just pile on to this one. Here it is, pasted over:

Do you want to request a feature or report a bug?

A bug, I'm sorry to say. (But it's not exactly your bug ... as will soon become apparent.)

What is the current behavior?

To the best of my understanding, during DOM updates, ReactDOM looks at document.activeElement to try and determine the element that currently has focus on the page. Unfortunately, in new versions of Safari, calling document.activeElement, when the active element belongs to an iframe with a different origin than the parent page, appears to throw an uncatchable error:

image

Here is a minimal reproduction in a JSFiddle: https://jsfiddle.net/Luktwrdm/1484/

In Safari, open your console, then click on the "Click to play" button (which will call setState() periodically), and then click into the embedded iframe.

You'll see the setState calls triggering the error above.

But it's really Safari's bug, not yours. If you then switch into JSFiddle's results frame, with text in the iframe selected, any attempt at reading document.activeElement will trigger the error ... and also return the frame:

image

And the error cannot be caught (getActiveElement is already doing this):

image

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Safari 12 and React 16 trigger this error. I'm not certain about previous versions of React or Safari.

Suggested fix?

Ideally, Safari would quickly release a new version that fixes this error for activeElement. But if that doesn't happen — I'm not sure if there's any way to safely use document.activeElement in Safari currently.

@koenbok
Copy link

koenbok commented Jan 27, 2019

Here is a working version of @asci version too: https://codesandbox.io/s/ojrq78r87q

This is the last thing holding us back from upgrading Framer to 16.8 cc @gaearon

Also, hi @jashkenas, always nice to run into you, even if it's a bug :-)

@philipp-spiess
Copy link
Contributor

@asci

to check iframe src origin before accessing contentDocument

while (element instanceof win.HTMLIFrameElement && element.src.indexOf(location.origin) === 0) {

If it's good we can create PR

I'm not sure. It is possible to create (and use) iframes that have no src property, e.g. via:

const frame = document.createElement('iframe');
const frameDocument = frame.contentDocument;
frameDocument.open();
frameDocument.write(html);
frameDocument.close();

Or even via absurd things like this one:

const frame = document.createElement('iframe');
frame.src = "javascript:<html><body>Hi!</body></html>";

And of course Object URLs and I don't know what else. We'd probably need a much more robust check to find out if the frame is cross domain.

All of that said, this is only about an error in the dev console, right? The execution is not limited since it will still throw an error (besides logging the annoying error in the console) which would allow our fallback logic to run and function correctly.

@philipp-spiess philipp-spiess mentioned this issue Mar 13, 2019
Closed
@renanvalentin renanvalentin mentioned this issue Mar 19, 2019
Merged
@gaearon
Copy link
Collaborator

gaearon commented Mar 28, 2019

Fix should be out in 16.8.6.

@mb21 mb21 mentioned this issue Apr 29, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Browser: Safari Component: DOM Type: Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jashkenas @koenbok @philipp-spiess @asci @gaearon