Closed Bug 1746448 (CVE-2022-29914) Opened 1 year ago Closed 10 months ago

Reuse popup window using same windowName able to overlap fullscreen notification

Categories

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

defect

Tracking

()

VERIFIED FIXED
101 Branch
Tracking Status
firefox-esr91 100+ verified
firefox99 --- wontfix
firefox100 + verified
firefox101 + verified

People

(Reporter: sourc7, Assigned: edgar)

Details

(Keywords: csectype-spoof, sec-high, Whiteboard: [reporter-external] [client-bounty-form][adv-main100+][adv-esr91.9+])

Attachments

(4 files, 1 obsolete file)

Attached file reusepopup.html (obsolete) —

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
Flags: sec-bounty?
Group: firefox-core-security → dom-core-security
Component: Security → DOM: Core & HTML
Product: Firefox → Core

I wasn't able to reproduce on OSX Nightly.

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

Flags: needinfo?(echen)

(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.

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.

Assignee: nobody → echen
Flags: needinfo?(echen)
Severity: -- → S2
Priority: -- → P2
Summary: Reuse popup window using same windowName able to overlap fullscreen notification → Reuse popup window using same windowName able to overlap fullscreen notification (not on mac)
Flags: needinfo?(dveditz)

(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.

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.

Flags: needinfo?(dveditz)
Summary: Reuse popup window using same windowName able to overlap fullscreen notification (not on mac) → Reuse popup window using same windowName able to overlap fullscreen notification
Attachment #9256805 - Attachment description: reusepopup.html → PoC (make spoofDelay longer to have this work on non-Windows platforms)
Attached file Bug 1746448; r?smaug

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.
Attachment #9271908 - Flags: sec-approval?

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

Approved to land and uplift

Attachment #9271908 - Flags: sec-approval? → sec-approval+
Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

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.
Attachment #9271908 - Flags: approval-mozilla-esr91?
Attachment #9271908 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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

Approved for 100.0b9

Attachment #9271908 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Approved for 91.9esr

Attachment #9271908 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
QA Whiteboard: [qa-triaged]
Flags: sec-bounty? → sec-bounty+

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

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form][adv-main100+][adv-esr91.9+]
Alias: CVE-2022-29914
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.