1833814 - (CVE-2024-1549) Custom Cursor mouse overlap permission request prompt (clickjacking on wrong button)

Status: VERIFIED FIXED

(Toolkit :: PopupNotifications and Notification Bars, defect)

Description

[Hafiizh]

Attachment: bandicam 2023-05-18 13-42-20-811.mp4

I found a vulnerability in firefox 112.0.1 (64-bit) (i tested on windows OS) where Mouse cursor pointer (128x128) can overlap the permissions request prompt which can confuse lead to spoofs.

step to reproduces:

1. open cursorpointer.html
2. click on webpage near the permission request prompt appears

Comment 1

[Hafiizh]

Attachment: cursorpointer.html

Comment 2

[Hafiizh]

Attachment: 128x128.png

Comment 3

[Hafiizh]

i update the poc:

1. open https://pipabajabakrie.com/upload/cursorpointer.html or open cursorpointer.html
2. click on webpage near the permission request prompt appears

Comment 4

[Daniel Veditz [:dveditz]]

i update the poc:

The pipabajabakrie.com server seems extremely unreliable from north america. Most of the time I get server not found or a 404 page (noticed in other testcases of yours as well). Can you please attach the update to this bug?

can overlap the permissions request prompt which can confuse lead to spoofs.

Can you explain or demonstrate what kind of spoof you mean? It doesn't seem like you can fool them into clicking on the prompt (e.g. fake cursor on one button with the real click going to the other) because it turns into the real cursor when you cross into the prompt. You could maybe fool someone into thinking they clicked on the prompt when they really clicked just off it, and then the prompt closes. But I don't see how that gives an attacker any advantage.

Comment 5

[Daniel Veditz [:dveditz]]

The permission dialogs are supposed to have an activation delay so you can't play "double-click game" clickjacking, and on mac that seems to hold true for the geolocation prompt used in the example.

I think I get it now (thanks, mccr8). I was moving my cursor too high and the modified one was going away.

The key to this kind of spoof is that the big cursor is usually transparent, with a normal-sized cursor somewhere other than the real pointer. By overlapping these chrome permission dialogs you could have someone click on the "wrong" button. You know users will deny geolocation (or camera, or installing an add-on) so you make an "offset cursor" such that when they try to click deny/block/cancel it will actually allow/approve/install. (you'd have to make a different cursor for different OSs because we change the button order)

We've had this problem a long time ago, especially with the add-on install prompt. The cursor must be reset whenever it overlaps ANY bit of chrome, including doorhanger bits. It cannot simply be based on the normal window border. If it's too much work to get the geometry correct I would accept cancelling custom cursors any time a door-hanger/permission prompt is visible.

Note also to test infobars, though we probably got that one correct.

We allow some grace for cursors <= 32x32, but for an offset cursor attack on these prompt that might still be too much overlap.

I have not tested the add-on install prompt, but I assume it will work the same as the other door-hangers. Based on that I'm rating it sec-high. It could also be sec-high if it worked on the getUserMedia() permission prompt (which I assume it will).

Comment 6

[Emilio Cobos Álvarez (:emilio)]

CSSOM seems like the wrong component for this. I guess if you see this as a core bug it could live in layout or DOM events, but it could also be fixed at the UI level.

In general detecting overlap with permission panels (specially from the content process) seems pretty hard, though presumably not impossible.

Comment 7

[:Gijs (he/him)]

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

CSSOM seems like the wrong component for this. I guess if you see this as a core bug it could live in layout or DOM events, but it could also be fixed at the UI level.

Can you elaborate on how you think it could/should be fixed "at the UI level"?

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I said could, not sure it's a great idea. But avoiding overlapping permission prompt actions on top of content is the obvious... Maybe moving the panels in a way the buttons end up around the urlbar height? It's arguably not great.

Comment 9

[:Gijs (he/him)]

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

Maybe moving the panels in a way the buttons end up around the urlbar height? It's arguably not great.

I don't think this is feasible given the panels' height, especially for getUserMedia (camera/microphone) panels.

Seems like Chrome solves this by doing what dveditz suggested:

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

If it's too much work to get the geometry correct I would accept cancelling custom cursors any time a door-hanger/permission prompt is visible.

Emilio, is there a primitive frontend can use for this? In my testing, just setting the CSS cursor property on the browser is insufficient. So that would presumably need an additional browsingContext or browser API of sorts... unless I'm missing something?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Does window.setCursor(..) work?

Comment 11

[:Gijs (he/him)]

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

Does window.setCursor(..) work?

window.setCursor("default") doesn't seem to do anything when called on the browser window and setCursor does not appear to exist for the content process window (tested w/ multiproc browser console set to the process scope for the tab and then tabs[0].content.setCursor which is undefined)

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Hmm, yeah, so setCursor("pointer") etc locks the cursor but not when over web content... It's only exposed to privileged chrome windows (https://searchfox.org/mozilla-central/rev/8dd0d2bebe4c897152da6c86d937e4be80bbaa54/dom/webidl/Window.webidl#639-640).

Probably we should make setCursor work on a privileged window but alternatively we could expose an API to disallow custom cursors temporarily, that should be rather straight-forward I believe...

Comment 13

[Paul Zühlcke [:pbz]]

Thanks for looking into it! Does either of you have time to take the bug or do you know somebody who could? I'm unfortunately quite swamped currently.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Same unfortunately, unless this is super-prioritary I can't take it right now. I can give pointers tho...

Ideally we expose a ChromeOnly API, let's call it window.allowCustomCursor which takes a boolean. On setting, we propagate the value to nsIWidget, via nsGlobalWindowOuter::GetMainWidget.

Then nsIWidget::SetCursor implementations take care of checking that and not honor the custom cursor?

Comment 16

[Paul Zühlcke [:pbz]]

Moving it out of Site Permissions since it's not permission prompt specific, but rather a general issue with popup notifications. Please let me know if that doesn't work!

Comment 17

[Paul Zühlcke [:pbz]]

Mike, you've recently reorganized the bug triages for notifications. Should this still be in Notifications and Alerts or is there a better place?

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Over to PopupNotifications and Notification Bars! :)

Comment 19

[Hafiizh]

hello any updates?

Comment 20

[Hafiizh]

hello any updates?

Comment 21

[Hafiizh]

hello any updates?

Comment 22

[:Gijs (he/him)]

Attachment: Bug 1833814 - change nsIWidget's cursor logic, r?emilio

Comment 23

[:Gijs (he/him)]

Comment on attachment 9367077 [details]
Bug 1833814 - change nsIWidget's cursor logic, r?emilio

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Reasonably easily. It's clear that there's some connection between the XUL popup manager code and custom cursors. Figuring out the link with permission prompts as such may be less obvious - I'm not sure.
• 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?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: It's a little iffy, I think, because we're messing with cursors and 3 different widget platforms, so there's a potential for races / edgecases. It's also not something we can really test automatically, unfortunately. So like - I hope not! But it's hard to be sure.
• Is Android affected?: No

Comment 24

[Tom Ritter [:tjr]]

Comment on attachment 9367077 [details]
Bug 1833814 - change nsIWidget's cursor logic, r?emilio

From a sec-approval POV, I'm okay with this landing now or at the beginning of next cycle. From the size of the patch and the things its touching, relman may want to wait until next cycle.

Comment 25

[:Gijs (he/him)]

(In reply to Tom Ritter [:tjr] from comment #24)

Comment on attachment 9367077 [details]
Bug 1833814 - change nsIWidget's cursor logic, r?emilio

From a sec-approval POV, I'm okay with this landing now or at the beginning of next cycle. From the size of the patch and the things its touching, relman may want to wait until next cycle.

Last beta is tomorrow. So yeah, let's wait for the next cycle I guess.

Comment 26

[:Gijs (he/him)]

Attachment: Bug 1833814 - change nsIWidget's cursor logic,

Original Revision: https://phabricator.services.mozilla.com/D195549

Comment 27

[Phabricator Automation]

Uplift Approval Request

• Code covered by automated testing: no
• Is Android affected?: no
• Fix verified in Nightly: no
• Steps to reproduce for manual QE testing: see comment 3 - in fixed builds, the custom cursor should be replaced with a normal mouse cursor as soon as the permission prompt appears (on Linux, Windows and macOS)
• Explanation of risk level: . The patch is fairly straightforward and there's a pref to turn things off if we get in a pickle. But unfortunately this isn't easily testable using automated tests
• User impact if declined: sec-high
• Needs manual QE test: yes
• Risk associated with taking this patch: Low to medium
• String changes made/needed: No

Comment 28

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

11 days ago, Gijs placed a reminder on the bug using the whiteboard tag [reminder-landing 2024-01-22] .

Gijs, please refer to the original comment to better understand the reason for the reminder.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2424d5c30267
change nsIWidget's cursor logic, r=emilio,win-reviewers,mhowell

Comment 30

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

https://hg.mozilla.org/mozilla-central/rev/2424d5c30267

Comment 31

[:Gijs (he/him)]

Attachment: Bug 1833814 - change nsIWidget's cursor logic, r?emilio

Original Revision: https://phabricator.services.mozilla.com/D195549

Comment 32

[Phabricator Automation]

Uplift Approval Request

• Steps to reproduce for manual QE testing: see comment 3 - in fixed builds, the custom cursor should be replaced with a normal mouse cursor as soon as the permission prompt appears (on Linux, Windows and macOS)
• Code covered by automated testing: no
• Is Android affected?: no
• Fix verified in Nightly: no
• User impact if declined: sec-high
• Explanation of risk level: The patch is fairly straightforward and there's a pref to turn things off if we get in a pickle. Also it's early in the beta cycle. But unfortunately this isn't easily testable using automated tests
• Needs manual QE test: yes
• Risk associated with taking this patch: Low
• String changes made/needed: None

Comment 33

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/190710fe87f9

Comment 34

[Daniel Bodea [:danibodea]]

I could not reproduce this issue in Nightly v124.0a1 from 2024-01-22, Beta v123.0b2 or in Release v122.0 in Windows 10, MacOS 11 or Ubuntu 22. The cursor does not change into a different shape or form and the permission dialog does not get covered in any way.

@Gijs: Do you have any idea why it wouldn't reproduce on my side? I have to mention that the test page in comment 3 is no longer available, while the attached test page in comment 1 would not show the starred rectangle at all. Thanks!

Comment 35

[:Gijs (he/him)]

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

I could not reproduce this issue in Nightly v124.0a1 from 2024-01-22, Beta v123.0b2 or in Release v122.0 in Windows 10, MacOS 11 or Ubuntu 22. The cursor does not change into a different shape or form and the permission dialog does not get covered in any way.

@Gijs: Do you have any idea why it wouldn't reproduce on my side? I have to mention that the test page in comment 3 is no longer available, while the attached test page in comment 1 would not show the starred rectangle at all. Thanks!

The testcase requires the 128x128 image file to live in the same directory (and probably requires accessing it through a webserver running either on https or localhost). I can try to get this up for you but currently fighting some webhosting issues and a few other fires. Let me know if you need this setting up for you or you can do it yourself?

Comment 36

[:Gijs (he/him)]

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

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

I could not reproduce this issue in Nightly v124.0a1 from 2024-01-22, Beta v123.0b2 or in Release v122.0 in Windows 10, MacOS 11 or Ubuntu 22. The cursor does not change into a different shape or form and the permission dialog does not get covered in any way.

@Gijs: Do you have any idea why it wouldn't reproduce on my side? I have to mention that the test page in comment 3 is no longer available, while the attached test page in comment 1 would not show the starred rectangle at all. Thanks!

The testcase requires the 128x128 image file to live in the same directory (and probably requires accessing it through a webserver running either on https or localhost). I can try to get this up for you but currently fighting some webhosting issues and a few other fires. Let me know if you need this setting up for you or you can do it yourself?

Figured my webhost out - https://gijsk.com/temp/cursorpointer.html .

Comment 37

[Daniel Bodea [:danibodea]]

Thank you for the help, Gijs!

I can confirm this reproduction in Release v122.0 as I could reproduce the behavior in the demo video. The fix can be verified in Beta v123.0.b2 and Nightly v124.0a1. The "starred rectangle" that covers the cursor and above is displayed just as long as a permission dialog appears; the rectangle won't cover up the permission dialog. Testing was performed in Windows 10, MacOS 11 and Ubuntu 22.

Comment 38

[Daniel Veditz [:dveditz]]

The security team thinks sec-moderate is more appropriate for this bug due to the user engineering an attack requires.

Comment 39

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/39193ec581e4

Comment 40

[Pascal Chevrel:pascalc]

Backed out 1 changesets (bug 1833814) for causing build failures on esr branch a=backout
https://hg.mozilla.org/releases/mozilla-esr115/rev/ca5f8275ec21

Gijs, the uplift to the esr115 branch is causing build failures:
https://treeherder.mozilla.org/jobs?repo=mozilla-esr115&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=39193ec581e438ea4e1583e8e00f1fb3b3e8981d&selectedTaskRun=WRJJQgZbRXOPZ6u8etdCLg.0

is there a dependent uplift missing for the branch maybe? Thanks

Comment 41

[Pascal Chevrel:pascalc]

Emilio, any idea why the ESR uplift is causing build bustage? Thanks

Comment 42

[:Gijs (he/him)]

Looks like on ESR nsXULPopupManager.cpp is missing the layout staticprefs include. I'll update the ESR patch.

Comment 43

[:Gijs (he/him)]

I've updated the patch, waiting on try to confirm it works.

Comment 44

[Phabricator Automation]

Uplift Approval Request

• Fix verified in Nightly: yes
• Code covered by automated testing: no
• String changes made/needed: no
• Steps to reproduce for manual QE testing: see comment 3 - in fixed builds, the custom cursor should be replaced with a normal mouse cursor as soon as the permission prompt appears (on Linux, Windows and macOS)
• User impact if declined: sec-moderate
• Is Android affected?: no
• Needs manual QE test: yes
• Risk associated with taking this patch: low to medium
• Explanation of risk level: The patch is fairly straightforward and there's a pref to turn things off if we get in a pickle. But unfortunately this isn't easily testable using automated tests

Comment 45

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/428ddcca4f91

Comment 46

[Daniel Bodea [:danibodea]]

The fix was also verified in ESR v115.8.0esr in Windows 10 and MacOS 11. The permission dialog cannot be covered by the "starred rectangle" in any way.

Comment 47

[Tom Ritter [:tjr]]

Attachment: advisory.txt