Closed Bug 1814597 (CVE-2023-29533) Opened 1 year ago Closed 1 year ago

Multiple requestFullScreen repeatedly then launch window with same window name able to overlap fullscreen notification toast

Categories

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

defect

Tracking

()

VERIFIED FIXED
113 Branch
Tracking Status
firefox-esr102 112+ verified
firefox111 --- wontfix
firefox112 + verified
firefox113 + verified

People

(Reporter: sourc7, Assigned: edgar)

References

Details

(Keywords: csectype-spoof, reporter-external, sec-high, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main112+][adv-esr102.10+])

Attachments

(4 files)

Attached file testcase.html

After launch new popup window then repeatedly call multiple requestFullscreen using setInterval and launch popup window with same windowName interestingly it able to overlap fullscreen notification toast.

I have tested this works on Linux X11 and Wayland.

Tested on:

  • Firefox Nightly 111.0a1 (2023-02-01) (64-bit) on Arch Linux KDE and Ubuntu 22.10
  • Firefox 109.0.1 (64-bit) on Arch Linux KDE

Steps to reproduce

  1. Visit attached testcase.html
  2. Click "newWindow" button
  3. Click "Spoof" button on Main Window
  4. Re-click "Spoof" if child window not maximized or browser not goes into full-screen
  5. Browser goes into full-screen and overlapped by popup window
Flags: sec-bounty?

It looks like related to my reported Bug 1798219 which involve fullscreen race condition + reuse popup window, this one is more reliable and straight-forward to reproduce.

Group: firefox-core-security → dom-core-security
Component: Security → DOM: Core & HTML
Product: Firefox → Core

Edgar, could you take a look? Thanks.

Flags: needinfo?(echen)

It's more plausible you could elicit this set of actions from a user. Not completely sold on the rating, but it does seem more "high" ish.

note: if, like me, you have disabled the ability to resize, move, and close windows this testcase won't work; you will have to revert those prefs. We really ought to make those the defaults for people -- no one's using those abilities for anything good.

Assignee: nobody → echen
Severity: -- → S2
Flags: needinfo?(echen)
Priority: -- → P2

There are two things need to check,
1). We don’t check user gestures on window.open() which is to open an existing window (with the same window name), but Chrome does, need to check how Webkit behaves and also spec.
2). I cannot reproduce the issue on macOS, seems like requesting fullscreen to cocoa widget will bring a background widget window to foreground, have not yet checked Windows.

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

2). I cannot reproduce the issue on macOS, seems like requesting fullscreen to cocoa widget will bring a background widget window to foreground, have not yet checked Windows.

Hmm, I think the reason that I could not reproduce this on macOS is because of the fullscreen transition,
https://searchfox.org/mozilla-central/rev/3ede9deb876ad5d6389cb51b371d4a4c8d788deb/modules/libpref/init/all.js#3381-3388, which delay the fullscreen request a bit and make the fullscreen request could switch focus back to the fullscreen window. If I turn the fullscreen transition off, I could also reproduce this on macOS.

In this case, window.open() which would bring the existing window get called before the requestFullscreen(), but parent process receive the request to raise the popup window after the fullscreen request, I think it is hard to ensure the request order that arrives parent process, but anyway, we should leave fullscreen if another window get raised.

FocusManager tries to handle that in https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/dom/base/nsFocusManager.cpp#1659-1687, but it doesn't work for this case. And it doesn't work for the case that window is raised by clicking link, either. We should fix them together.

Blocks: 1819756
Attached file Bug 1814597; r?smaug
Attachment #9321006 - Attachment description: WIP: Bug 1814597; → Bug 1814597; r?smaug

Comment on attachment 9321006 [details]
Bug 1814597; 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?: No
  • 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?: Should be safe as we have multiple existing tests cases for different scenarios, e.g.
  • Is Android affected?: No
Attachment #9321006 - Flags: sec-approval?

Comment on attachment 9321006 [details]
Bug 1814597; r?smaug

Approved to land and uplift.

Attachment #9321006 - Flags: sec-approval? → sec-approval+
Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch

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

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(echen)

Comment on attachment 9315566 [details]
Firefox - Multiple requestFullscreen repeatedly + reuse popup window name able to overlap fullscreen toast.mp4

Beta/Release Uplift Approval Request

  • User impact if declined: Fullscreen warning notification is overlapped by popup window, which could lead to address bar UI spoofing.
  • 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: Follow steps in 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): Should be safe as we have multiple existing tests cases for different scenarios, e.g.
  • String changes made/needed: None
  • Is Android affected?: No
Flags: needinfo?(echen)
Attachment #9315566 - Flags: approval-mozilla-beta?
Attachment #9321006 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Attachment #9315566 - Flags: approval-mozilla-beta?
Attachment #9321006 - Flags: approval-mozilla-esr102?

Comment on attachment 9321006 [details]
Bug 1814597; r?smaug

Approved for 112.0b3

Attachment #9321006 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Comment on attachment 9321006 [details]
Bug 1814597; r?smaug

Approved for 102.10esr

Attachment #9321006 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+

I was able to reproduce the bug using the STR from comment 0, on an affected Nightly build (2023-02-02). I've checked with macOS 11.

The issue is verified as fixed on latest Nightly 113.0a1 and Beta 112.0b4 under macOS 11, Win 10 x64 and Ubuntu 18.04 x64. I'll verify this on ESR 102.10 as well once the builds are available in treeherder.

Flags: sec-bounty? → sec-bounty+

This is also verified as fixed on Esr 102.10, under macOS 11, Win 11 and Ubuntu 18.04 x64.

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

Attachment

General

Created:
Updated:
Size: