1793829 - (CVE-2022-45408) Re-run window.open with same windowName allow focus stealing can be used to overlap fullscreen notification toast

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Irvan Kurniawan (:sourc7)]

Attachment: testcase.bundle.html

After launch child popup window with window.open then re-run window.open with same windowName the child window able to repeatedly gain focus without requiring user click activation. With that method I found the child window popup able to overlap another popup fullscreen notification toast.

When run this on mozregression it point to this pushlog:

17:09.62 INFO: Last good revision: 566f81bfa373512b41c1a47962e21a06078d7bf8 (2021-02-08)
17:09.62 INFO: First bad revision: 89c5f958a3ac4795109acf1f9dff1c8026bb82fe (2021-02-09)
17:09.62 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=566f81bfa373512b41c1a47962e21a06078d7bf8&tochange=89c5f958a3ac4795109acf1f9dff1c8026bb82fe

One of the pushlog that has window.open keyword is Take focus from window.open etc even if we're already active but not in the active window. On nsDocShell.cpp I've revert the shouldTakeFocus code to the old one, when re-run the window.open the child window wouldn't able to gain focus.

Tested on:

• Firefox 105.0.2 (64-bit) on Arch Linux (KDE X11)
• Firefox 105.0.2 (64-bit) on Arch Linux (KDE Wayland)
• Firefox 105.0.2 (64-bit) on Ubuntu 22.04.1 LTS (Wayland)
• Firefox Nightly 107.0a1 (2022-10-05) (64-bit) on Arch Linux (KDE X11)
• Firefox Nightly 107.0a1 (2022-10-05) (64-bit) on Arch Linux (KDE Wayland)

Steps to reproduce:

1. Visit attached testcase.bundle.html on Linux OS
2. Click "Launch stealFocusWindow"
3. Click "Launch requestFullscreenWindow"
4. Click "requestFullscreen" on child popup window
5. Fullscreen notification toast will be overlapped with child window

Comment 1

[Irvan Kurniawan (:sourc7)]

Attachment: Firefox - Rerun window.open able to steal focus to overlap fullscreen notification toast.mp4

Comment 2

[Daniel Veditz [:dveditz]]

Emilio: this looks like a regression from bug 1691214; could you take a look?

On a quick try I couldn't reproduce on Mac, but I didn't dig into whether I have any settings that might be interfering.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1691214

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Security Approval Request

• How easily could an exploit be constructed based on the patch?: somewhat? The fix is kind of obvious.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: Bug 1691214
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Should graft cleanly.
• How likely is this patch to cause regressions; how much testing does it need?: not likely. Again, the fix is obvious-ish.
• Is Android affected?: No

Comment 6

[Emilio Cobos Álvarez (:emilio)]

ni? to write a test when I have some time.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1793829 - Add a test. r=hsivonen,peterv

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Approved to land and uplift after beta branches

Comment 9

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

Don't steal focus for navigations without user activation. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/71625a0f7411d2ec0f199f8c9b621713059be6bf
https://hg.mozilla.org/mozilla-central/rev/71625a0f7411

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Rather simple tweak.
• String changes made/needed: none
• Is Android affected?: No

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Approved for 107.0b3.

Comment 13

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/54a90dd928ee

Comment 14

[Rares Doghi, Desktop QA]

Verified as fixed in our latest Nightly Build as well as Beta 107.0b3.

Comment 15

[Donal Meehan [:dmeehan]]

:emilio this grafts cleanly to esr102, if you could submit an ESR uplift request when ready

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Beta/Release Uplift Approval Request

• User impact if declined: Comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively trivial fix to avoid stealing focus in some cases without a user interaction.
• String changes made/needed: none
• Is Android affected?: No

Comment 17

[Donal Meehan [:dmeehan]]

Comment on attachment 9297417 [details]
Bug 1793829 - Don't steal focus for navigations without user activation. r=hsivonen

Approved for 102.5esr.

Comment 18

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/8bfe53ec3269

Comment 19

[Rares Doghi, Desktop QA]

Verified as fixed in our latest Esr 102.5.

Comment 20

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

3 months ago, Tom Ritter [:tjr] placed a reminder on the bug using the whiteboard tag [reminder-test 2023-01-01] .

emilio, please refer to the original comment to better understand the reason for the reminder.

Comment 22

[Emilio Cobos Álvarez (:emilio)]

Guess I can land the test now?

Comment 23

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Yes!

Comment 24

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

Add a test. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/6dccc823484fe867a8026470fb3891a99c83af0f
https://hg.mozilla.org/mozilla-central/rev/6dccc823484f