1432856 - (CVE-2020-6810) Popup window can cover fullscreen security UI

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Avi Drissman]

Attachment: fullscreenpopuptest.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3315.3 Safari/537.36

Steps to reproduce:

Hello. I am Avi Drissman on the Chrome security team. While investigating a security issue in Chrome, I found that Firefox is also vulnerable. The issue is https://crbug.com/776418 . While I'm happy to allow any Firefox security people access to that bug, I will explain the issue entirely in this bug.

When you take a page in Chrome into fullscreen, a bubble is shown at the top of the screen, saying "hey, you're in fullscreen, tap the escape key to get out". Chrome considers this to be security UI. Firefox has a similar bubble, and I imagine you consider this bubble to be security UI as well.

Obscuring that bubble would be bad. Therefore, if a page tries to open a popup while it is fullscreen, Chrome drops fullscreen. Firefox has the same behavior, and I again imagine it is for the same reason.

However, suppose a page opens a popup while not in fullscreen. If the page is subsequently taken to fullscreen, and then the page uses window.focus() to focus the popup, the popup is shown on top of the fullscreened page, and it obscures the fullscreen security UI bubble.

I attach a sample HTML page that will allow you to reproduce this behavior.

The original Chrome bug report has a very elaborate reproduction that spoofs Chrome UI to induce the user to click on the original page. Please be assured that limits on user gestures will not help here.

The solution that Chrome will be taking will be to kick a page out of fullscreen if it attempts to do any focusing of popups. Edge already has this behavior, and so I recommend that Firefox adopt this behavior.

In addition, I'm proposing this behavior to be standardized; see https://github.com/whatwg/fullscreen/issues/116 .

Please feel free to contact me ([e-mail]/[e-mail]) with any questions you may have.

Comment 1

[Daniel Veditz [:dveditz]]

We have a bug on making window.focus() kick us out of fullscreen. Was used as an abuse technique--this fullscreen thing is new to us. When I find that again I'll make this bug depend on it.

Comment 2

[Daniel Veditz [:dveditz]]

Actually we have conflicting bugs about window.focus() and fullscreen, but the one I was vaguely remembering is not this bug.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Daniel Veditz [:dveditz] from comment #2)
> Actually we have conflicting bugs about window.focus() and fullscreen, but
> the one I was vaguely remembering is not this bug.

I guess you were thinking about bug 1196588 and probably bug 724554.

(In reply to Avi Drissman from comment #0)
> Created attachment 8945145 [details]
> fullscreenpopuptest.html

This test page is great. Thanks!

> Obscuring that bubble would be bad. Therefore, if a page tries to open a
> popup while it is fullscreen, Chrome drops fullscreen. Firefox has the same
> behavior, and I again imagine it is for the same reason.

It doesn't seem to me that we do, at least I cannot reproduce this behavior on Windows. There may be some other behavior on other platforms due to certain code path, but I believe it's not intended.

It's not clear to me how the mechanism you proposed would work, or specifically, where should we put the check in when someone invokes window.focus(). I guess there are several options:
(1) kick out any fullscreen document when anyone invokes an effective window.focus()
(2) kick out any fullscreen document of an origin when another window of that origin is raised by an effective window.focus()
(3) kick out fullscreen document when anything in the stack calling into window.focus() is related to that document

So, 3 is complicated, and not enough, because one can always ask other document to do this in an async way, although that may be rejected by the popup abuse protection we have for window.focus(), but nothing in the spec really stops it from happening.

(1) and (2) seem to be too strict to me. I think there are valid usecase for a fullscreen document to open a popup or something like that, e.g. when you are doing presentation, and you want to show another page in a new window.

IIUC, the problem we want to solve is that, the security UI should not be blocked by anything when we show it. In that sense, I think we can solve it in an easier way:
* when we about to show the security UI, we check that we do have the focus, if not, kick the document out from fullscreen, and
* when the notification is on screen, if we lose the focus, kick the document out from fullscreen.

That should solve the security concern here without adding much complexity to the platform logic, I believe. And the spec just need to make developers aware in the section of "Security and Privacy Considerations".

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Daniel, what do you think about the solution I proposed at the end of comment 3?

Comment 5

[Daniel Veditz [:dveditz]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> > the one I was vaguely remembering is not this bug.
> 
> I guess you were thinking about bug 1196588 and probably bug 724554.

I think I was thinking of conversations the Security Engineering team has had about Chrome's Intent to Implement
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/8Z36QLPkCd0/xrGkjvLcAAAJ

But also see bug 1437219

> (2) kick out any fullscreen document of an origin when another window of
> that origin is raised by an effective window.focus()

Restricting it to the same origin wouldn't stop abuse -- easy to make sure the other window is on a different but confederate domain.

> IIUC, the problem we want to solve is that, the security UI should not be
> blocked by anything when we show it. In that sense, I think we can solve it
> in an easier way:
> * when we about to show the security UI, we check that we do have the focus,
> if not, kick the document out from fullscreen, and
> * when the notification is on screen, if we lose the focus, kick the
> document out from fullscreen.

That sounds great.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

ni? myself for implementing this.

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I am not sure if Xidorn has bandwidth to implement this since he's not full-time employee.
NI Ethan to put this into the security team's radar.

Comment 8

[Ethan Tseng [:ethan]]

Johann, this is an issue that our team can handle?

Comment 10

[Johann Hofmann [:johannh]]

Paul might be able to give it a shot, but to set expectations this is more of a backlog item for us. I'll add it to the list :)

Comment 11

[Ethan Tseng [:ethan]]

(In reply to Johann Hofmann [:johannh] from comment #10)

Paul might be able to give it a shot, but to set expectations this is more of a backlog item for us. I'll add it to the list :)

Sounds good. Thanks!

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Thanks! I indeed may not have bandwidth to work on this myself at the moment, but I'm always happy to answer any question and possibly review the code, so feel free to flag me if there's anything I can help!

Comment 13

[Paul Zühlcke [:pbz]]

IIUC, the problem we want to solve is that, the security UI should not be
blocked by anything when we show it. In that sense, I think we can solve it
in an easier way:

• when we about to show the security UI, we check that we do have the focus,
  if not, kick the document out from fullscreen, and
• when the notification is on screen, if we lose the focus, kick the
  document out from fullscreen.

I've done some testing and also implemented this behaviour by listening for the blur event. However this can interfere with normal usage. For example, opening a Video in fullscreen and then doing something on a secondary monitor would cause the browser to leave fullscreen.

I think the approach Chromium uses, to leave fullscreen on window.open() and also on window.focus() might be a good solution. Alternatively we could only behave this way while the fullscreen warning is shown.

Comment 14

[Xidorn Quan [:xidorn] UTC+10]

I think the approach Chromium uses, to leave fullscreen on window.open() and also on window.focus() might be a good solution.

It might be a good solution but I also think there can be reasonable usecase triggering this. A imaginary case, for example, if user is watching a sample movie in fullscreen, and when it comes to part requiring payment, the website may pop up a payment window, and when the payment finishes, user can continue the movie in fullscreen.

Alternatively we could only behave this way while the fullscreen warning is shown.

That's exactly what I meant.

Comment 15

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Exit full-screen on window.open() and window.focus() when fs security warning is visible. r=smaug

Comment 16

[Paul Zühlcke [:pbz]]

Apologies for the delayed update on this. I've looked into various options and also submitted a patch, but I'm not happy with it yet. Only leaving fullscreen when the fs warning is on screen requires a lot of refactoring, since we only know about the fs warning state in the frontend. The event has to be passed all the way through the focus manager to the frontend code. This includes extending the webidl interfaces to pass caller type (if the open/focus was triggered by the website or the system).
Going for Chromiums approach and exiting fullscreen on window.open and window.focus, would make for a much cleaner patch. We could handle everything in the backend. It would also help with other 'evil traps' where websites abuse popups in fullscreen to trick users into installing addons (Bug 1596189).

While there are some valid use cases for popups in fullscreen this wouldn't fully break workflows. Websites are aware of the fullscreen state and can avoid this behavior; given the market share of Chromium based browsers they already do. An example use case is Google Slides, where the user can open speaker notes in a popup. In Chromium, if the notes window is opened while in (DOM) fullscreen, the fullscreen is cancelled and the user has to enter it again. That's an extra click.

Comment 17

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Leave DOM fullscreen on window.open(). r=smaug

Comment 18

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Leave DOM fullscreen on window raise. r=smaug

Depends on D55809

Comment 19

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Extended focus methods in Window.webidl, Client.webidl and Element.webidl to pass CallerType. r=smaug

Depends on D55810

Comment 20

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Added prefs for DOM fullscreen leave on window open or raise. r=smaug

Depends on D55811

Comment 21

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Added console logging for DOM fullscreen leave on window raise. r=smaug

Depends on D55812

Comment 22

[Johann Hofmann [:johannh]]

That seems reasonable to me. Thank you for all the work, Paul.

Comment 23

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Added tests for window open and focus in fullscreen. r=smaug

Depends on D55813

Comment 24

[Paul Zühlcke [:pbz]]

Attachment: Bug 1432856 - Disable fullscreen leave on raise and open for fullscreen-multiple test. r=smaug

Depends on D56391

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustages failures in nsWindow.h:

https://hg.mozilla.org/integration/autoland/rev/d94afce0c18b72e8febfaa68a21fcb7790d1118c

Build failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=281441289&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=3d08c3cce53325d6623178e60d1ca5d38b4721c5
/builds/worker/workspace/build/src/widget/cocoa/nsChildView.h:310:16: error: 'SetFocus' marked 'override' but does not override any member functions
/builds/worker/workspace/build/src/widget/cocoa/nsChildView.h:282:7: error: abstract class is marked 'final' [-Werror,-Wabstract-final-class]
/builds/worker/workspace/build/src/widget/cocoa/nsChildView.h:310:16: error: 'nsChildView::SetFocus' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
/builds/worker/workspace/build/src/widget/cocoa/nsCocoaWindow.h:235:16: error: 'SetFocus' marked 'override' but does not override any member functions
/builds/worker/workspace/build/src/widget/cocoa/nsCocoaWindow.h:207:7: error: abstract class is marked 'final' [-Werror,-Wabstract-final-class]
/builds/worker/workspace/build/src/widget/cocoa/nsCocoaWindow.h:235:16: error: 'nsCocoaWindow::SetFocus' hides overloaded virtual function [-Werror,-Woverloaded-virtual]

Eslint failure: https://treeherder.mozilla.org/logviewer.html#?job_id=281436170&repo=autoland
browser/base/content/test/fullscreen/open_and_focus_helper.html:32:37 | Prefer boolean length check (mozilla/prefer-boolean-length-check)

Had landed before as
https://hg.mozilla.org/integration/autoland/rev/a303b775a51b3df084ccd88d457ed2c0f8c7a4a3
https://hg.mozilla.org/integration/autoland/rev/8351a8b1d96ad8ff5b501d94b3b2c76c79db4293
https://hg.mozilla.org/integration/autoland/rev/c81f3d5b9bf33e648b9d7b4e427d1c9a8a10c38e
https://hg.mozilla.org/integration/autoland/rev/a8a4fa63f5b20f2dcc0b11e0496076e435c7e83c
https://hg.mozilla.org/integration/autoland/rev/62fc84c8ce9978f4df6d65ac6e8d8e4939b0bb65
https://hg.mozilla.org/integration/autoland/rev/49d03dd89b1759bdf28f50ad5330d5a2f9651803
https://hg.mozilla.org/integration/autoland/rev/3d08c3cce53325d6623178e60d1ca5d38b4721c5

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/325eec16f032c255567b1081f941735677d9c5b2
https://hg.mozilla.org/integration/autoland/rev/f09a7b5da021be3b821e571fd8cb27db8d72dc63
https://hg.mozilla.org/integration/autoland/rev/5cb2b4727452b54b0ecf07c10178e01e1f8bc4a1
https://hg.mozilla.org/integration/autoland/rev/10fde12558b7cf7fb1f2849d8b2bdbf3b5b196c9
https://hg.mozilla.org/integration/autoland/rev/7594dab35ba4fcb5ce33e58cfae265aa76787f8d
https://hg.mozilla.org/integration/autoland/rev/03b7899094663bba35802764ac8f15d9f28f69da
https://hg.mozilla.org/integration/autoland/rev/a03b869d45a9680b63d45d96f217c2c476b20d09
https://hg.mozilla.org/mozilla-central/rev/325eec16f032
https://hg.mozilla.org/mozilla-central/rev/f09a7b5da021
https://hg.mozilla.org/mozilla-central/rev/5cb2b4727452
https://hg.mozilla.org/mozilla-central/rev/10fde12558b7
https://hg.mozilla.org/mozilla-central/rev/7594dab35ba4
https://hg.mozilla.org/mozilla-central/rev/03b789909466
https://hg.mozilla.org/mozilla-central/rev/a03b869d45a9

Comment 27

[Ryan VanderMeulen [:RyanVM]]

I see no reason to rush this fix to release. LMK if you feel strongly otherwise.

Comment 28

[Johann Hofmann [:johannh]]

Dan, Tom suggests in bug 1596189 comment 17 that we should open up this bug and I think it makes sense. The patch is public and I think since the projected attack is mostly about spoofing or irritating the user, there's no risk of an "active exploit" appearing in the small window of time until the fix hits release. It made sense to keep it sensitive while the time to fix was unknown but now I don't think we're at risk.

If you agree, can you open this up, please?

Comment 29

[Daniel Bodea [:danibodea]]

I have attempted verification with the "fullscreenpopuptest.html" attachment using the following steps:

1. Open browser.
2. Load the attachment (login to Bugzilla required).
3. Make sure that the browser window is on the primary screen (if more than one is being used).
4. Click on the "Make pop-up" button.
5. Click on the "Toggle fullscreen" button.
6. Click on the "Focus pop-up" button.
   Affected: The pop-up window is displayed and overlaps the full-screen related notification.
   Fixed: The pop-up window DOES NOT overlap the full-screen related notification. However, the user is forced to leave full-screen mode.

These are my results on Nightly v74.0a1 from 2020-01-31:

• Windows 10: The pop-up window DOES NOT overlap the full-screen related notification. However, the user is forced to leave full-screen mode. Also, in this case, the pop-up does not stay in focus because the normal window gets focused after leaving full-screen mode.
• Ubuntu 18.04: The pop-up window DOES NOT overlap the full-screen related notification. However, the user is forced to leave full-screen mode. Also, in this case, the pop-up will stay in focus (as seen in the Chrome browser).
• Mac OS 10.14.6: The pop-up window DOES NOT overlap the full-screen related notification. However, the user is forced to leave full-screen mode. Also, in this case, the pop-up does not stay in focus because the normal window gets focused after leaving full-screen mode.

Should a new bug be logged for the secondary issue seen on Windows 10 and Mac OS?

Comment 30

[Daniel Bodea [:danibodea]]

Hsin-Tsai, I am requesting information from you in hope that you could answer some questions regarding the fix. Which are:
One issue that I observed is that the browser will instantly leave the full-screen mode when the pop-up is focused, but I am assuming that this is the compromise we are making in favor of not covering the notification about exiting full-screen. Can you confirm this is a valid approach?
A second issue is that the pop-up that gets focused will lose its focus because the "normal" browser window will be focused when leaving full-screen mode. This issue occurs only on Windows and Mac OS. Is this a valid issue? Should it be logged?

Comment 31

[Paul Zühlcke [:pbz]]

Thanks for verifying the fix!

(In reply to Bodea Daniel [:danibodea] from comment #30)

Hsin-Tsai, I am requesting information from you in hope that you could answer some questions regarding the fix. Which are:
One issue that I observed is that the browser will instantly leave the full-screen mode when the pop-up is focused, but I am assuming that this is the compromise we are making in favor of not covering the notification about exiting full-screen. Can you confirm this is a valid approach?

Yes, this is the approach we opted for. See discussion above.

A second issue is that the pop-up that gets focused will lose its focus because the "normal" browser window will be focused when leaving full-screen mode. This issue occurs only on Windows and Mac OS. Is this a valid issue? Should it be logged?

Yes, that's a bit inconsistent. The behavior on Linux is the correct one, we should honor the focus request, so the window should be in focus after leaving fullscreen. I've filed Bug 1613065.

Comment 33

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 34

[Tom Ritter [:tjr]]

Attachment: advisory.txt