1746448 - (CVE-2022-29914) Reuse popup window using same windowName able to overlap fullscreen notification

Status: VERIFIED FIXED

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

Description

[Irvan Kurniawan [:sourc7]]

Attachment: reusepopup.html

After launching a popup in the same windowName Firefox will reuse the same popup window instead of launching a new one. Interestingly when combining this method with requestFullscreen the reused popup window able to show in top of the fullscreen notification.

Mozregression show it is regression of Bug 1712038 - Exit fullscreen only when the focus change will raise the window. On the good build Firefox will exit the fullscreen with following console.warn Exited fullscreen because a window was focused.

Steps to reproduce:

1. Visit attached reusepopup.html
2. Click "Launch Popup"
3. Click "Reuse Popup Spoof"
4. Fullscreen notification will be overlapped with reused popup window

Comment 1

[Irvan Kurniawan [:sourc7]]

Attachment: Firefox - Reuse Popup to Overlap Fullscreen Notification.mp4

Comment 2

[Andrew McCreight [:mccr8]]

I wasn't able to reproduce on OSX Nightly.

Comment 3

[Andrew McCreight [:mccr8]]

Edgar, when you are back could you take a look? Thanks.

Comment 4

[Irvan Kurniawan [:sourc7]]

(In reply to Andrew McCreight [:mccr8] from comment #2)

I wasn't able to reproduce on OSX Nightly.

I able to reproduce this on Windows 11 and Arch Linux KDE, but on Ubuntu 21.01 it prevent the window stealing and show OS toast "Mozilla Firefox is ready" instead of switch to the child window.

I forgot to mention to click "Reuse Popup Spoof" in the main window instead of child window, therefore I will attach the revised testcase to hide the button in the child window.

Comment 5

[Irvan Kurniawan [:sourc7]]

Attachment: PoC (make spoofDelay longer to have this work on non-Windows platforms)

Comment 6

[Edgar Chen [:edgar]]

I could not reproduce on OSX, either, it seems somehow focus would switch back to the window that requests fullscreen on OSX.

I think the main point is that when the window.open would reuse the existing window, we don't check the user activation. Compared to Chrome, it blocks the window.open if there is no valid user activation (in this case, requestFullscreen() call consumes the transient user activation) even if an existing window is reused. I think we could just follow Chrome's behavior which also makes more sense.

Comment 7

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar] from comment #6)

I think the main point is that when the window.open would reuse the existing window, we don't check the user activation. Compared to Chrome, it blocks the window.open if there is no valid user activation (in this case, requestFullscreen() call consumes the transient user activation) even if an existing window is reused. I think we could just follow Chrome's behavior which also makes more sense.

I checked the spec again, spec doesn't check the user activation if the existing window is reused, see https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name. And Safari doesn't check the user activation, either.

After a second thought, even we follow Chrome's behavior, the issue isn't fixed completely, a user could still bring the existing window foreground via window.open (if it is called within a valid user activation), so it seems we should exit fullscreen in this case instead. I also check Chrome and Safari, both of them exit fullscreen after window.open would bring an existing window foreground.

Comment 8

[Daniel Veditz [:dveditz]]

There were two reasons I couldn't reproduce this on Mac:

1. I forgot I had disabled moveTo() and resizeTo() because they are used for evil (pref dom.disable_window_move_resize)
2. the spoofDelay was too short. When changed to the same as Windows it worked like Windows

[We should] exit fullscreen after window.open would bring an existing window foreground.

Or exit fullscreen if window.open() is called, regardless of its effects. Also if any other window gets focus or is raised (we already do that, right?)

Eliciting two clicks isn't all that hard if you can catch the user's attention -- the first click is the bigger hurdle. This isn't as clean as previous ways to slip into fullscreen unnoticed, but I'm reluctantly calling it sec-high anyway because sec-moderate undersells it.

Comment 9

[Edgar Chen [:edgar]]

Attachment: Bug 1746448; r?smaug

Comment 10

[Edgar Chen [:edgar]]

Comment on attachment 9271908 [details]
Bug 1746448; r?smaug

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The patch likely makes it obvious what the issue might be.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: I would expect that this patch could apply to all branches directly.
• How likely is this patch to cause regressions; how much testing does it need?: I think it is unlikely to cause a regression, it only affect the case that window.open would reuse a existing window, and other browser behave the same.

Comment 11

[Tom Ritter [:tjr]]

Comment on attachment 9271908 [details]
Bug 1746448; r?smaug

Approved to land and uplift

Comment 12

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

r=smaug
https://hg.mozilla.org/integration/autoland/rev/4a033d94dbf5eb85632c0ef649681689c3522c26
https://hg.mozilla.org/mozilla-central/rev/4a033d94dbf5

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 9271908 [details]
Bug 1746448; r?smaug

Beta/Release Uplift Approval Request

• User impact if declined: Fullscreen notification might be hidden by a popup window and user might be spoofed.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Follows the STR in https://bugzilla.mozilla.org/show_bug.cgi?id=1746448#c0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I think it is unlikely to cause a regression, it only affect the case that window.open would reuse a existing window, and other browser behave the same.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug.
• User impact if declined: Fullscreen notification might be hidden by a popup window and user might be spoofed.
• Fix Landed on Version: 101
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I think it is unlikely to cause a regression, it only affect the case that window.open would reuse a existing window, and other browser behave the same.

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9271908 [details]
Bug 1746448; r?smaug

Approved for 100.0b9

Comment 15

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f9e8783bbbed

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9271908 [details]
Bug 1746448; r?smaug

Approved for 91.9esr

Comment 17

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/de5f87f2171d

Comment 18

[Ciprian Georgiu, Desktop QA]

I've managed to reproduce this bug using STR from comment 0, on Ubuntu 18.04 x64 and Win 11 (on an affected Nightly build, 97.0a1, 20211206215435).

The issue is verified as fixed on the latest builds: 91.9.0 Esr, 100.0 Release and 101.0a1 Nightly

Comment 19

[Frederik Braun [:freddy]]

Attachment: advisory.txt