requestPointerLock not called from short running user-generated event handler leads to Across Tab Website Clickjacking
Categories
(Core :: DOM: Events, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
thunderbird_esr78 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox84 | --- | wontfix |
firefox85 | + | verified |
firefox86 | + | verified |
People
(Reporter: sourc7, Assigned: edgar)
References
(Regression)
Details
(4 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][sec-survey][adv-main85+])
Attachments
(4 files, 1 obsolete file)
685 bytes,
text/html
|
Details | |
3.99 MB,
video/mp4
|
Details | |
48 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
tjr
:
sec-approval+
|
Details | Review |
221 bytes,
text/plain
|
Details |
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:
- Visit attached clickjack-github.html
- Click "Start Clickjacking on Github"
- Wait until Github fully loaded
- Click "Let's start!"
- 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)
Reporter | ||
Comment 1•4 years ago
|
||
Comment 2•4 years ago
|
||
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.
Reporter | ||
Comment 3•4 years ago
|
||
(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.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 4•4 years ago
|
||
Comment 5•4 years ago
|
||
Edgar, would you have time to take a look? If not, assign back to me :)
Assignee | ||
Comment 6•4 years ago
•
|
||
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.
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Assignee | ||
Comment 8•4 years ago
|
||
(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•4 years ago
|
||
Probably not quite sec-high
, but sec-moderate
doesn't seem adequate.
Assignee | ||
Comment 10•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 11•4 years ago
|
||
Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;
Approved to land and request uplift
Comment 12•4 years ago
|
||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Ni for the uplift request to get this into 85 beta? :-)
Assignee | ||
Comment 14•4 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Comment 15•4 years ago
|
||
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.
Assignee | ||
Comment 16•4 years ago
|
||
(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•4 years ago
|
||
Comment on attachment 9195743 [details]
Bug 1684837 - Adjust pointer lock request handling;
approved for 85.0b9
Comment 18•4 years ago
|
||
uplift |
Updated•4 years ago
|
Comment 19•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 20•4 years ago
|
||
Comment 21•4 years ago
|
||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•8 months ago
|
Updated•8 months ago
|
Description
•