1916152 - (CVE-2024-11698) Can't Exit Full Screen (if "always ask you where to save files" is turned on)

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Kang Ali]

Attachment: firefox_repro.mov

Steps to reproduce:

Open Firefox
open file index.html
click Start Fullscreen and Hidden Download
save file or cancel
locked exit fullscreen ( can u press esc or right click to back or anything can exit full screen )

Testing :

Firefox Version : 129.0.2 (64-bit)
OS : MacOS Monterey

Comment 1

[Kang Ali]

Attachment: index.html

Comment 2

[:Gijs (he/him)]

Olli, is this something you can investigate while Edgar is out? Unclear to me if this is macOS-specific.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I don't really have time

Comment 4

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

:sfarre is going to investigate a bit.

Comment 5

[Simon Farre [:sfarre]]

:farre has reproduced this on mac; and also ran the current WIP for fullscreen ipc which at least at the moment made it not reproducible.

Edit: well apparently it seems to intermittently fail, so at this moment it's safe to assume that I need to dig further into what causes this specific issue.

Comment 6

[Kang Ali]

any update for this issue ?

Thanks

Comment 7

[Andrew McCreight [:mccr8]]

I believe that work is ongoing for bug 1913666, which it sounds like should fix this problem.

Comment 8

[Simon Farre [:sfarre]]

(In reply to Kang Ali from comment #6)

any update for this issue ?

Thanks

I've just started investigating this more seriously and it seems as though this behavior is only reproducible if the screen is not already maximized/fullscreen. Can you verify this to be true on your machine as well? So, steps to reproduce (without failing) should be:

1. Go fullscreen
2. Click the button "Start Fullscreen and Hidden download"

It's possibly something with the transition that plays a part here

Comment 9

[Kang Ali]

after im testing in firefox nightly version 133.0a1 (2024-10-21) (64-bit)

this cannot reproduce again

Comment 10

[Simon Farre [:sfarre]]

(In reply to Kang Ali from comment #9)

after im testing in firefox nightly version 133.0a1 (2024-10-21) (64-bit)

this cannot reproduce again

That's interesting, I can reliably reproduce it on the very tip of master, but if I first go fullscreen before clicking the button it does not cause the buggy behavior.

Did you try both ways?

Comment 11

[Kang Ali]

Attachment: repro_update_firefox.mp4

im testing in Firefox 131.0.3 (64-bit)
this can reproduce

this video update to go fullscreen before clicking

Comment 12

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

It looks like this might be something related to "native" fullscreen on macOS. Could you also try to reproduce the issue with setting full-screen-api.macos-native-full-screen to false? Thanks!

Comment 13

[Simon Farre [:sfarre]]

Attachment: xcode test application

I've investigated this further and I am almost 100% certain that it is the combination of being in native fullscreen transition & opening of a modal dialog, that causes this behavior.

After having wrote a simple test application that has a button that makes the application go fullscreen while also opening a modal save-as window, clicking it forces the application out of fullscreen/prevents the application to complete fullscreen transition and then showing the modal dialog.

I think this is the key to what happens in gecko/firefox, because we don't account for this issue, at all, so we think we're in some fullscreen state while the OS knows us to be in another state and this causes the UI hang.

Test application that reproduces the behavior of being kicked out of fullscreen transition when opening a modal dialog is in the attachment.

The simplest solution I can think of is that we probably need to have some sort of guarding behavior, where we check "are we currently in native fullscreen transition; if so, post a runnable that opens the panel upon completion" or something like that.

Comment 14

[Simon Farre [:sfarre]]

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

It looks like this might be something related to "native" fullscreen on macOS. Could you also try to reproduce the issue with setting full-screen-api.macos-native-full-screen to false? Thanks!

Yes this is definitely something to do with native fullscreen on macOS, see my previous comment. The default behavior seems to be this: if you open a modal dialog while being in fullscreen transition, the transition does not complete. I don't think we account for this behavior at all.

Comment 15

[Simon Farre [:sfarre]]

Also, sorry that I was unclear: I did test it with native turned off (the pref set to false) and it indeed did not cause this behavior.

Comment 16

[Simon Farre [:sfarre]]

We could also just flat out deny opening a save-as panel while being in native transition, this would be even easier.

Comment 17

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

(In reply to Simon Farre [:sfarre] from comment #14)

Yes this is definitely something to do with native fullscreen on macOS, see my previous comment. The default behavior seems to be this: if you open a modal dialog while being in fullscreen transition, the transition does not complete. I don't think we account for this behavior at all.

Thanks for the investigations and tests! Then we might need some input from :bradwerth. :)

:bradwerth, It looks like opening a modal dialog during native fullscreen transition would make the transition not complete, is this an issue and should be fixed in widget side? Thanks!

Comment 18

[Kang Ali]

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

It looks like this might be something related to "native" fullscreen on macOS. Could you also try to reproduce the issue with setting full-screen-api.macos-native-full-screen to false? Thanks!

yeah i tried with set full-screen-api.macos-native-full-screen to false and it can exit fullscreen

Comment 19

[Brad Werth [:bradwerth]]

I'll try to figure this out.

Comment 20

[Brad Werth [:bradwerth]]

Probably the same issue as Bug 1893997.

Comment 21

[Simon Farre [:sfarre]]

(In reply to Brad Werth [:bradwerth] from comment #20)

Probably the same issue as Bug 1893997.

Possibly could be. But initial experimentation with MacOS native fullscreen transition and usage of modal dialogs, seem to suggest that it simply isn't supported. If you create a default application in XCode and use the attached code in my comment, the OS kicks the application out of fullscreen instantly (the application does the following: toggle fullscreen for the window and opens a save-as panel just like the one we use in Firefox). This seems to suggest that it's not natively supported to do both at the same time. This would also explain why this bug isn't reproduced at all when the window first is fullscreened before opening the modal dialog.

I tried experimenting with a sample cocoa application that used non-modal dialogs and these seemed to not cause this kicking-out behavior, so there seem to be a very specific edge case around modal dialogs and native fullscreen transitions for cocoa applications.

Comment 22

[Simon Farre [:sfarre]]

(In reply to Brad Werth [:bradwerth] from comment #20)

Probably the same issue as Bug 1893997.

I've checked out that issue again and they seem unrelated; that issue seem to involve the ESC key being dispatched to do more things than to just what's currently focused and it should actually be able to be reproduced on all platforms if my suspicion is correct, it's just that MacOS is the only platform (I don't think Windows has it) where you can get a toolbar while being in fullscreen. The issue is caused by a method in PresShell, that suggests it's intercepted before being dispatched to (for instance) the about boxes while being in fullscreen, see here. We check for a fullscreen element and call AsyncExitFullscreen which will kick out all fullscreened documents.

Asking around online (not mentioning the origin application, us, that is), I seem to get 3rd party verification that indeed they see the same issue (when building & running the example application I've provided here).

Comment 23

[Brad Werth [:bradwerth]]

Good news is we've got a way to detect this problem. In a local patch, I've confirmed that this situation causes the NSWindow delegate method windowDidFailToEnterFullScreen to be fired. We currently ignore it, but this is a case where we'll need to do "the right thing" to handle this unusual case. I'll provide a patch soon.

Comment 24

[Brad Werth [:bradwerth]]

I've confirmed this is only possible with native fullscreen.

Comment 25

[Brad Werth [:bradwerth]]

Attachment: Bug 1916152: Make nsCocoaWindow handle failing to enter or exit fullscreen.

Previously, we had relied on macOS eventually invoking the
windowDidEnterFullscreen and windowDidExitFullscreen delegate methods.
But in cases where macOS does not, and instead invokes the "fail"
methods, we can do the right thing to put the nsCocoaWindow in a
synchronized state with the DOM.

Comment 26

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

Thanks, then move to Widget: Cocoa for now. Feel free to let us know if there’s anything we can help with on the DOM side!

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4d85c2b5322
Make nsCocoaWindow handle failing to enter or exit fullscreen. r=mac-reviewers,mstange

Comment 28

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

https://hg.mozilla.org/mozilla-central/rev/b4d85c2b5322

Comment 29

[Kang Ali]

after RESOLVED
this report eligable bounty or cve ?

Thanks

Comment 30

[Donal Meehan [:dmeehan]]

:bradwerth could you add uplift requests for beta and esr128 when you have a moment?

Comment 31

[Brad Werth [:bradwerth]]

Comment on attachment 9434115 [details]
Bug 1916152: Make nsCocoaWindow handle failing to enter or exit fullscreen.

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Users may be unable to exit fullscreen properly under unusual conditions.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This adding an existing response to an OS event that we were ignoring. The code invoked is used for similar purposes in response to other events we were already responding to.
• String changes made/needed:
• Is Android affected?: No

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: A safe, small fix that could improve user experience in unusual conditions.
• User impact if declined: Users may be unable to exit fullscreen properly under unusual conditions.
• Fix Landed on Version: 134
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This adding an existing response to an OS event that we were ignoring. The code invoked is used for similar purposes in response to other events we were already responding to.

Comment 32

[Donal Meehan [:dmeehan]]

Comment on attachment 9434115 [details]
Bug 1916152: Make nsCocoaWindow handle failing to enter or exit fullscreen.

Approved for 133.0b4

Comment 33

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/85858bd57fae

Comment 34

[Raluca Popovici, Desktop QA]

The issue is still reproducible on Fx 129.0.2 and Fx 131.0.3 using MacOS 13.2.1.
It was verified as fixed on Fx 133.0b4(treeherder build) and Nightly 134.0a1(2024-11-04) using MacOS 13.2.1.

Comment 35

[Donal Meehan [:dmeehan]]

Comment on attachment 9434115 [details]
Bug 1916152: Make nsCocoaWindow handle failing to enter or exit fullscreen.

Approved for 128.5esr.

Comment 36

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/47de48e111df

Comment 37

[Raluca Popovici, Desktop QA]

It was verified as fixed on Fx 128.5.0(treeherder build) using MacOS 13.2.1.

Comment 38

[Kang Ali]

this report eligable bounty or cve ?

Thanks

Comment 39

[Daniel Veditz [:dveditz]]

DOS bugs are generally not eligible for bug bounties and unfortunately this one is not an exception. We will issue an advisory with a CVE for it, and enter your name on our "hall of fame" pages.

Comment 40

[Kang Ali]

Oke thanks for CVE

Please add name for CVE Advisory : Kang Ali of Punggawa Cybersecurity

Thanks

Comment 41

[Tom Ritter [:tjr]]

Attachment: advisory.txt