Custom cursor goes outside the bounds of the web content in "malware" website
Categories
(Core :: Layout, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | verified |
People
(Reporter: bignis, Assigned: emilio)
References
(Blocks 2 open bugs)
Details
(4 keywords, Whiteboard: [adv-main67+])
Attachments
(5 files)
Comment 1•7 years ago
|
||
Comment 4•7 years ago
|
||
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
Reporter | ||
Comment 10•7 years ago
|
||
Comment 11•7 years ago
|
||
Comment 12•7 years ago
|
||
Updated•7 years ago
|
Comment 13•6 years ago
•
|
||
Comment 14•6 years ago
|
||
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
Assignee | ||
Comment 18•6 years ago
|
||
Assignee | ||
Comment 19•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 21•6 years ago
|
||
I'm taking a look at this, will start with some preliminar refactoring.
Comment 22•6 years ago
|
||
Do folks have opinions on the different approaches here? Chrome has a tentative plan that we like here https://bugs.chromium.org/p/chromium/issues/detail?id=880863#c41 (comment 41), but it's best if we can align on a solution.
Assignee | ||
Comment 23•6 years ago
|
||
(In reply to csharrison from comment #22)
Do folks have opinions on the different approaches here? Chrome has a tentative plan that we like here https://bugs.chromium.org/p/chromium/issues/detail?id=880863#c41 (comment 41), but it's best if we can align on a solution.
I think clipping the cursor is not great because the majority of the cursor may actually be transparent, and clipping the non-transparent area leaves the user without any kind of feedback...
I did a bunch of refactoring in bug 1520502 to be able to show the right cursor when intersecting the browser chrome, though I'm of course open to implementing something else :)
Assignee | ||
Comment 24•6 years ago
|
||
And for now, start blocking custom cursors that are larger than 64 pixels.
I wish I knew how to add a test for this, but I tested manually using the
test-case from bug 1518189 with and without hidpi.
We always treat the cursor image size as CSS pixels (and upscale it to device
pixels in HiDPI). We have bugs to stop doing that though (bug 1425694).
Comment 25•6 years ago
|
||
Comment 26•6 years ago
|
||
bugherder |
Comment 28•6 years ago
|
||
Hey Emilio, sorry I missed this. In the Blink intent-to-deprecate I was planning on blocking custom cursors >32x32, but I now see you've done this only for >64x64 cursors.
Our usage data in Chrome 74 shows ~.01% of page visits impacted by the 32x32 scoping (i.e. only those pages have the cursors go outside of the viewport boundary). What do you think about making this more strict?
Comment 29•6 years ago
|
||
(In reply to csharrison from comment #28)
Hey Emilio, sorry I missed this. In the Blink intent-to-deprecate I was planning on blocking custom cursors >32x32, but I now see you've done this only for >64x64 cursors.
Our usage data in Chrome 74 shows ~.01% of page visits impacted by the 32x32 scoping (i.e. only those pages have the cursors go outside of the viewport boundary). What do you think about making this more strict?
+ni to radar this.
Assignee | ||
Comment 30•6 years ago
|
||
I'm ambivalent, it seems ok to do that. It's a one-liner patch1.
I'm curious which kind of attacks can you do with cursors smaller than 64px though...
Comment 31•6 years ago
|
||
It's not much, but at least in Chrome you can just barely get to the "x" button on a tab with 63px if the bookmark bar is hidden, and you can easily reach the extension buttons / back / forward / etc. If the limit is 32x, you can barely reach even those controls.
My slight preference is to protect more and revert to gating on >64x64 if we find compat issues.
Comment 32•6 years ago
|
||
I'm OK with erring on the side of caution and going with 32x so we have alignment. (Looks like we would based on this commit: https://chromium.googlesource.com/chromium/src/+/0acc6c91a8bce9237aa8c3a5085945339f50e150.)
I don't think we will want to ship Fx67 with the 64x limit and Fx68 with the 32x limit, however, so I guess we either get the change in during this week's soft freeze or do an uplift.
Assignee | ||
Comment 33•6 years ago
|
||
(In reply to Sean Voisen (:svoisen) from comment #32)
I'm OK with erring on the side of caution and going with 32x so we have alignment. (Looks like we would based on this commit: https://chromium.googlesource.com/chromium/src/+/0acc6c91a8bce9237aa8c3a5085945339f50e150.)
I don't think we will want to ship Fx67 with the 64x limit and Fx68 with the 32x limit, however, so I guess we either get the change in during this week's soft freeze or do an uplift.
I disagree with that, that chromium patch only adds a deprecation message, as far as I understand, so we'd be the only ones actually blocking the cursor.
Blocking 32px cursors adds significantly more risk. I'm fine landing that change on nightly, but probably not on 67... wdyt?
Assignee | ||
Comment 34•6 years ago
|
||
Ah, nvm, the actual removal landed in https://chromium.googlesource.com/chromium/src.git/+/a03681911775122dbb69690bcb025f0c3401ce04%5E%21/
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 37•6 years ago
|
||
Is there any plan to do something more thorough here, or is this fix as far as it will go?
Assignee | ||
Comment 38•6 years ago
|
||
(In reply to Eric Shepherd [:sheppy] from comment #37)
Is there any plan to do something more thorough here, or is this fix as far as it will go?
Can you elaborate? I don't think there's any plan to add any more restrictions.
Comment 39•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #38)
(In reply to Eric Shepherd [:sheppy] from comment #37)
Is there any plan to do something more thorough here, or is this fix as far as it will go?
Can you elaborate? I don't think there's any plan to add any more restrictions.
+ni to radar to :sheppy.
![]() |
||
Comment 40•6 years ago
|
||
This issue is not 100% fixed with doorhanger notifications.
I just tried it with the latest nightly (Build ID 20190322101035): https://i.imgur.com/4aQeyBz.mp4
(The cursor does not really flash like in the video, that's from the recording software.)
As you can see the custom cursor goes many pixels into the doorhanger.
This is from the following malicious website (still live):
hxxps://s3.us-east-2.amazonaws.com/rsscfr/de/index.html
(malicious XPI here: hxxp://s3.us-east-2.amazonaws.com/exyyt/de.xpi
)
Assignee | ||
Comment 41•6 years ago
|
||
Can you file a separate bug for that?
That's a quite different situation, and the fix for that would be fairly different (and probably way more complicated).
Note that the content process has no idea of whether there's a door-hanger around and what's its geometry. Also, how doorhangers and fullscreen interact in that site seems unfortunate...
![]() |
||
Comment 42•6 years ago
|
||
Filed bug 1538402.
Updated•6 years ago
|
Comment 43•6 years ago
|
||
FYI, similar nonsense going on at https://s3.us-east-2.amazonaws.com/usgvgv/us/index.html
HAR capture attached
reported to Amazon
Comment 44•6 years ago
|
||
Sorry for adding the link, I tried to defang with \ but apparently Bugzilla just ignored the escapes.
Comment 45•6 years ago
|
||
Note to MDN writers:
I've added a note about this to the Fx67 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#CSS
In terms of other work, I think it makes sense to add a note about Fx/Chrome's change to the BCD for the cursor property?
Updated•6 years ago
|
Comment 46•6 years ago
|
||
What I meant earlier about "do we plan to do something more thorough" is this: the idea of just limiting the cursor size to a specific pixel maximum feels placeholder-ish to me, for a couple of reasons:
-
32-pixel cursors are not very large; many games use larger cursors than this for various features, for example, and that feels very much like a serious restriction.
-
Using a pixel count maximum is especially problematic in a HiDPI or UltraDPI setting. a 32x32 pixel cursor on an extremely high resolution device is tiny.
-
It's just... not a very clean solution.
It seems like we could do some actual checking into the cursor image itself. Look at the cursor and look for things such as, perhaps:
-
Is the hotspot on a visible pixel which is part of a group of N[1] visible pixels, or within a small number of pixels of such a group? If not, is there a grouping of at least N visible pixels in every direction from the hotspot? This will prevent the hotspot from being off in a total void.
-
Along every edge of the cursor, there should be at least N pixels in a group that contacts the edge. There's no reason a cursor should have empty space on any side.
-
It might even make sense to crop images so that every edge has a grouping of N pixels making contact with each edge of the image.
[1] N is a computed number of pixels that, grouped together, are reasonably easily visible to the average person given the current display resolution.
Comment 47•6 years ago
|
||
Documentation updated:
- Submitted BCD PR 4111 to update data for Firefox and Firefox Android (which doesn't support
cursor
at all) - Added a paragraph to CSS cursor property page that says that browsers may choose to set a size limit
That should cover it (once the PR is merged and the production site updated).
Comment 48•6 years ago
|
||
I've managed to reproduce this issue on an affected Firefox 61.0a1 (2018-03-14) using Windows 10x64 by following the STR from comment 0.
This issue is verified fixed using Firefox 67.0b19 and Firefox 68.0a1(2019-05-12) on the following OSes: Windows 10x64, mac OS 10.14, Ubuntu 18.04x64 and Windows 7x64.
The top bar of Firefox is working fine except the part with the fake cursor on the page.
Comment 49•6 years ago
|
||
Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2019/custom-cursor-larger-than-32px-will-be-ignored-when-intersecting-ui/
Updated•6 years ago
|
Comment 50•6 years ago
|
||
FYSA, encountered this again on http[:]//errorcode21210.mac-us209[.]live (TorBrowser 60.8.0) , javascript disabled.
I have HAR capture available if necessary.
The page also causes a denial-of-service condition in the latest FF 68.0.2 (bug 1575952)
Description
•