1865914 - PopupNotifications clickjacking by obscuring notification via full screen exit transition

Status: VERIFIED FIXED

(Toolkit :: PopupNotifications and Notification Bars, defect, P1)

Description

[haxatron1]

Attachment: test.html

By combining techniques from https://bugzilla.mozilla.org/show_bug.cgi?id=1839073 and https://bugzilla.mozilla.org/show_bug.cgi?id=1857430, possible to bypass fullscreen.

STR
0. For better verification, increase security.notification_enable_delay in about:config to 1000.

1. Open test.html
2. Hover over button and spam click.
3. The prompt is accepted instantaneously after a fullscreen transition without enablement delay of 1000ms

Hypothesis: In this patch we reset the timeShown of https://hg.mozilla.org/mozilla-central/rev/75b8d8d5ea21 when window containing prompt gains focus. Therefore, I think any modifications to the timeShown variable in the fullscreen patch can be reset though this.

Comment 1

[haxatron1]

Attachment: bypass.webm

Comment 2

[haxatron1]

Correction: By combining techniques from https://bugzilla.mozilla.org/show_bug.cgi?id=1839073 and https://bugzilla.mozilla.org/show_bug.cgi?id=1857430, possible to bypass the security enablement delay for permission prompt

Comment 3

[:Gijs (he/him)]

(In reply to haxatron1 from comment #0)

By combining techniques from https://bugzilla.mozilla.org/show_bug.cgi?id=1839073 and https://bugzilla.mozilla.org/show_bug.cgi?id=1857430, possible to bypass fullscreen.

AFAICT you don't have access to 1857430. How did you obtain the testcase from that bug?

Comment 4

[:Gijs (he/him)]

Paul, can you take a look? My suspicion is this means the delay from 1857430 isn't really helping.

Comment 5

[haxatron1]

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

(In reply to haxatron1 from comment #0)

By combining techniques from https://bugzilla.mozilla.org/show_bug.cgi?id=1839073 and https://bugzilla.mozilla.org/show_bug.cgi?id=1857430, possible to bypass fullscreen.

AFAICT you don't have access to 1857430. How did you obtain the testcase from that bug?

Guessed from the information in https://www.mozilla.org/en-US/security/advisories/mfsa2023-49/#CVE-2023-6206. (This is what I usually do)

Comment 6

[Andrew McCreight [:mccr8]]

I don't know how distinct it is from those prior bugs, as I haven't looked into it, and it seems a lot harder to pull off than bug 1839073 because the user has to keep clicking for a lot longer, but I guess I'll mark it as sec-high like the others for now.

Comment 7

[Emma Zühlcke [:emz]]

I first suspected that the extended timeShown value that is set on full screen exit here would get overwritten by the PopupNotifications code when reshowing the panel here - but that's not the case. We don't trigger the full screen exit due to the permission prompt showing. We trigger it because "[...] fullscreen element was removed from document." in which case we don't extend the security delay. This full screen exit is called here.

(1) As a short term fix we could extend the security delay for all full screen exits and make sure we don't overwrite it by setting timeShown = Math.max( timeShown ?? 0, performance.now() ) here and here. This may be annoying for users for legitimate use cases where sites exit full screen and show a permission prompt.

(2) As a more holistic, long term fix we could extend the security delay whenever the button is pressed during the security delay. That means if a user spam clicks we effectively keep extending the delay until they don't interact with the button for a while. That makes these spam click clickjack attacks less effective. We could build something based on EnableDelayHelper currently used for prompts.
Chrome seems to have a similar behavior.

Gijs, what do you think about these two approaches? Should we do both? (2) doesn't seem good to uplift. (1) might be okay to uplift, but may be annoying for users.

Comment 8

[Emma Zühlcke [:emz]]

Update:

(1) Is quite hard to get right, since at the time of the _showPanel call of PopupNotifications this.window.document.fullscreenElement isn't true. I also tried listening for full screen events but so far I haven't found a good callback that works with the PoC. I also think we might have to listen for DOM full screen in all windows since a popups full screen transition may also be used to obscure the permission prompt.

For (2) I prototyped integrating the EnableDelayHelper into PopupNotifications and adding checks for mouseup during security delay to extend it. That seems to work well when I manually test it, but not for the PoC. Currently debugging this.

Comment 9

[Emma Zühlcke [:emz]]

I was confused because full screen transition is significantly longer longer than expected. We seem to be hitting https://searchfox.org/mozilla-central/rev/24985943a2e482c4be8e0ca7fabacb3674b91f25/dom/base/nsGlobalWindowOuter.cpp#4114-4126.

Comment 10

[:Gijs (he/him)]

(In reply to Paul Zühlcke [:pbz] from comment #8)

Update:

(1) Is quite hard to get right, since at the time of the _showPanel call of PopupNotifications this.window.document.fullscreenElement isn't true. I also tried listening for full screen events but so far I haven't found a good callback that works with the PoC. I also think we might have to listen for DOM full screen in all windows since a popups full screen transition may also be used to obscure the permission prompt.

Maybe the DOM full screen parent actor could send observer notifications?

For (2) I prototyped integrating the EnableDelayHelper into PopupNotifications and adding checks for mouseup during security delay to extend it. That seems to work well when I manually test it, but not for the PoC. Currently debugging this.

I think either way uplifting this or both of these and then shipping whatever we do right before Christmas/NY has got trouble written all over it. If we screw up, there won't be many people around to do anything about it. I think we may still want to do both, but I wouldn't worry about uplift until we figure out how complex either/both approaches are and how long it takes to get them polished up.

(In reply to Andrew McCreight [:mccr8] from comment #6)

it seems a lot harder to pull off than bug 1839073 because the user has to keep clicking for a lot longer, but I guess I'll mark it as sec-high like the others for now.

Should we downgrade to sec-moderate given the length of time people are required to keep clicking? No matter what we do, if we assume that the user will keep clicking indefinitely, there is always going to be a trade-off around making it harder to clickjack and also still allowing people to use their computer - if I'm in 5 "google meet" meetings or whatever each day, I'm going to be pretty quick to smash that "allow" button, and if we extend any disabling delays too long then that won't work and "normal users" in benign usecases will be frustrated.

Comment 11

[Andrew McCreight [:mccr8]]

Sure.

Comment 12

[Emma Zühlcke [:emz]]

Attachment: Bug 1865914 - Part 1, r=Gijs!

Comment 13

[Emma Zühlcke [:emz]]

Attachment: Bug 1865914 - Part 2, r=Gijs!

Depends on D194801

Comment 14

[Emma Zühlcke [:emz]]

I've attached two patch drafts and I'll submit another one tomorrow:

1. Extending the sec delay whenever a click is made during the sec delay. This helps against "spam click this button" attacks, because the button will never work unless the user stops for ~500ms before clicking again.
2. Ensure that we don't overwrite an already extended security delay, e.g. if we extended during a full screen transition.
3. Add observer messages for fullscreen-transition-start and fullscreen-transition-end so the PopupNotifications code can know whether we're in a full screen transition and extend the security delay accordingly. We can also use this to defer showing the panel in the first place.

Comment 15

[Emma Zühlcke [:emz]]

Attachment: Bug 1865914 - Part 3, r=Gijs!

Depends on D194802

Comment 16

[Emma Zühlcke [:emz]]

Attachment: Bug 1865914 - Test, r=Gijs!

Depends on D194886

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ade0563b39e
Part 1, r=Gijs
https://hg.mozilla.org/integration/autoland/rev/98d591466be0
Part 2, r=Gijs
https://hg.mozilla.org/integration/autoland/rev/d81d28de3c54
Part 3, r=Gijs,edgar

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/8ade0563b39e
https://hg.mozilla.org/mozilla-central/rev/98d591466be0
https://hg.mozilla.org/mozilla-central/rev/d81d28de3c54

Comment 23

[haxatron1]

Before this bug gets assessed for reward, it seemed like in #c6, https://bugzilla.mozilla.org/show_bug.cgi?id=1839073 was brought up as a basis to justify a downgrade in severity to sec-moderate.

However, I would also like to point out that https://bugzilla.mozilla.org/show_bug.cgi?id=1857430 was rated as sec-high (at least, according to release notes). I am not sure on the exact details of that bug, but I think that bug should also be used as a second basis to compare to for this particular bug.

Comment 24

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

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

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

For more information, please visit BugBot documentation.

Comment 26

[Emma Zühlcke [:emz]]

Comment on attachment 9365857 [details]
Bug 1865914 - Part 3, r=Gijs!

Beta/Release Uplift Approval Request

• User impact if declined: Clickjacking issue tricking users into accepting sensitive permission promps such as camera, microphone and geolocation.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0. May not be reproducible on macOS since the full screen transition works differently there.
  Beyond the STR on this bug, it would be good to do additional QA on permission prompts also in combination with website full screen (e.g. via https://permission.site). We should make sure that these patches don't introduce regressions where permission prompts can no longer be accepted / rejected, particularly when the website is not in full screen mode. The patches add an extended security delay when notifications show during entering or leaving full screen. This extended delay means the permission prompt buttons can not be interacted with for at least ~2.5 seconds. This extended delay should not apply when no full screen is involved.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Part 1 and 2 are low risk.
  Part 3 is medium risk. We keep track of full screen transition state, if we get that flag wrong, e.g. we missed and edge case where a transition ends but we don't update the window variable, we risk that popup notifications / permission prompts always have a ~2500ms delay before users can interact with them. Also outside of full screen (transitions).
• String changes made/needed:
• Is Android affected?: No

Comment 27

[Emma Zühlcke [:emz]]

Comment on attachment 9365857 [details]
Bug 1865914 - Part 3, r=Gijs!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bug has multiple duplicates, two reporters found the sec issue independently. The fix we landed in 122 (and uplifting into 121) makes the sec issue pretty obvious.
• User impact if declined: See beta uplift request
• Fix Landed on Version: 122
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Medium risk, see beta uplift request

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9365676 [details]
Bug 1865914 - Part 1, r=Gijs!

I'm a bit nervous about taking these so late in the cycle with only a couple weeks of bake time before shipping, but it sounds like we kind of have to. Let's hope QA is able to get some testing in. Approved for 121.0b9 and 115.6esr.

Comment 29

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/654107b0deaa
https://hg.mozilla.org/releases/mozilla-beta/rev/0f3086959e8e
https://hg.mozilla.org/releases/mozilla-beta/rev/4b4492de7d4c

Comment 30

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/37e0413888be
https://hg.mozilla.org/releases/mozilla-esr115/rev/c79aa699229a
https://hg.mozilla.org/releases/mozilla-esr115/rev/8e1a7b5b734b

Comment 31

[Emma Zühlcke [:emz]]

NI for landing the test.

Comment 32

[Phabricator Automation]

Comment on attachment 9366265 [details]
Bug 1865914 - Test, r=Gijs!

Revision D195096 was moved to bug 1879855. Setting attachment 9366265 [details] to obsolete.

Comment 33

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter

Comment 34

[Ina Popescu, Desktop QA]

I've replicated this issue with Firefox 120.0 on Windows 10 x64 following the STR from Comment 26.
Verified as fixed in the latest Nightly 127.0a1 and Firefox 126.0b7 versions on Windows 10 x64, macOS 13, and Ubuntu 22.04, as the issue no longer occurs.