1684837 - (CVE-2021-23955) requestPointerLock not called from short running user-generated event handler leads to Across Tab Website Clickjacking

Status: VERIFIED FIXED

(Core :: DOM: Events, defect, P2)

Description

[Irvan Kurniawan (:sourc7)]

Attachment: clickjack-github.html

When page request for pointer lock that not called inside short running user-generated event handler, Firefox on Desktop unintentionally pass every mouse event into that page (even on another tab). Surprisingly I notice it affect entire tab session, so after opening the page (that requestpointerlock), then open another website on the same tab, it still pass every mouse event into that tab (even on another tab).

As a result, it is possible to clickjack the target website by opening target website into that tab. I've crafted the payload to demonstrate clickjacking on Github website.

It is nearly universal clickjacking, because when testing it able to clickjack on following popular website:

• https://www.google.com
• https://www.amazon.com
• https://bugzilla.mozilla.org
• https://www.facebook.com
• https://www.youtube.com

It's strange on some website (e.g. https://twitter.com) the mouse event no longer passed to that tab.

Version affected:

• Firefox 84.0.1 (64-bit)
• Firefox Nightly 86.0a1 (2021-01-03) (64-bit)

Version unaffected:

• Firefox 78.6.0esr (64-bit)

Steps to reproduce:

1. Visit attached clickjack-github.html
2. Click "Start Clickjacking on Github"
3. Wait until Github fully loaded
4. Click "Let's start!"
5. Mouse click is pass to Github star button (If you're logged in, you've star or unstar the microsoft/vscode repositories, otherwise you'll redirected to Github login page)

Comment 1

[Irvan Kurniawan (:sourc7)]

Attachment: Firefox for Desktop - Github Clickjacking PoC.mp4

Comment 2

[:Gijs (he/him)]

This seems bad from a very casual look - Olli, can you investigate, or pass this to a more appropriate person? I don't know much about our pointer lock implementation (and it's not really a frontend thing, I think?).

Marking regression keywords based on comment 0 indicating ESR isn't affected.

Comment 3

[Irvan Kurniawan (:sourc7)]

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

Marking regression keywords based on comment 0 indicating ESR isn't affected.

Yes, it's a regression, I can only reproduce this in Firefox 83 and later versions.

Comment 4

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

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f82548378b796659a01ffc797be5319183de8a15&tochange=45eb5295b99bb22925db3355781b35cd5c1ce653

Comment 5

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

Edgar, would you have time to take a look? If not, assign back to me :)

Comment 6

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

After bug 1662587, content process will send a message to the parent process to request pointer lock and wait for parent process response, I suspect we don't handle some cases properly, for example, the page that requests pointer lock lose the focus before getting the response from parent process. I will look into this closely tomorrow.

Comment 7

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1684837 - Adjust pointer lock request handling;

Comment 8

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

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

After bug 1662587, content process will send a message to the parent process to request pointer lock and wait for parent process response, I suspect we don't handle some cases properly, for example, the page that requests pointer lock lose the focus before getting the response from parent process. I will look into this closely tomorrow.

It turns out it is not the case, the issue is that content process that requests the pointer lock doesn't tell parent process the request is denied in some case, so parent process still keep redirecting the mouse event to that content process.

Comment 9

[Daniel Veditz [:dveditz]]

Probably not quite sec-high, but sec-moderate doesn't seem adequate.

Comment 10

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I don't think it is easy to construct an exploit based on the patch.
• 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?: 83
• If not all supported branches, which bug introduced the flaw?: Bug 1662587
• 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?: Should be low, the patch adjusts the flow but keep most of the logic. There are existing mochitest/wpt tests for pointer lock, I also have a local test for this issue, but it would be good to play a bit on the gaming site.

Comment 11

[Tom Ritter [:tjr]]

Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;

Approved to land and request uplift

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40e9698f5ca3

Comment 13

[:Gijs (he/him)]

Ni for the uplift request to get this into 85 beta? :-)

Comment 14

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;

Beta/Release Uplift Approval Request

• User impact if declined: If a page requests a pointerlock but failed, the subsequent mouse events could possibly be dispatched to the wrong page.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: STR is in https://bugzilla.mozilla.org/show_bug.cgi?id=1684837#c0
• 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 low, the patch adjusts the flow but keep most of the logic. There are existing mochitest/wpt tests for pointer lock. But since it is a security bug the test for this issue isn't landed, I only have a local test for this change.
• String changes made/needed: None

Comment 15

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 16

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #15)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Done.

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;

approved for 85.0b9

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eb0aea4994f9

Comment 19

[Ciprian Georgiu, Desktop QA]

I was able to reproduce this bug using the steps from comment 0, on an affected Nightly build (2021-01-04).

The bug doesn't reproduce anymore on the latest builds: Nightly 86.0a1 and Beta 85.0b9. I have tested this on Win 10 x64, mac 10.15 and Ubuntu 18.04 x64.

Comment 20

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 21

[Frederik Braun [:freddy]]

Attachment: advisory.txt