Closed Bug 1408702 Opened 2 years ago Closed 2 years ago

Resist fingerprinting causes scrollbar glitch in Firefox 58

Categories

(Core :: Layout, defect, P2)

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: lola_j22, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [tor][fingerprinting-breakage])

Attachments

(2 files)

Attached image error.png
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171014100219

Steps to reproduce:

Turn on privacy.resistFingerprinting in latest Nightly


Actual results:

The scrollbar glitches with both arrows on both sides of each scrollbar.


Expected results:

The option should not cause a scrollbar error.
Component: Untriaged → Layout
Product: Firefox → Core
Flags: needinfo?(tihuang)
Assignee: nobody → tihuang
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: regression
Priority: -- → P2
Whiteboard: [tor][fingerprinting]
Whiteboard: [tor][fingerprinting] → [tor][fingerprinting-breakage]
Duplicate of this bug: 1409847
Given the regression range in bug 1409847 comment 0, this seems to be a regression from bug 1405311.
Blocks: 1405311
Flags: needinfo?(emilio)
Assignee: tihuang → emilio
Flags: needinfo?(emilio)
To be clear, privacy.resistFingerprinting for media features was broken (in the sense that it was useless, because you could use :-moz-system-metric(..) before bug 1405311).

Bug 1405311 changed internal uses of :-moz-system-metric to use media queries, which do honor that pref, and thus the chrome code that styled scrollbars broke with that pref set.

I still need to go through the rest of media features unship them from content pages, but the scrollbars and such were already unshipped so this patch fixes the issue observed here.
Flags: needinfo?(tihuang)
Emilio, thanks for verifying and fixing this.
Does this mean that by fixing this bug the resist fingerprinting feature is slightly compromised?
(In reply to Slew from comment #6)
> Does this mean that by fixing this bug the resist fingerprinting feature is
> slightly compromised?

No, nobody except chrome code can access those media queries anymore.
(In reply to Slew from comment #6)
> Does this mean that by fixing this bug the resist fingerprinting feature is
> slightly compromised?

Also, to elaborate comment 4: before bug 1405311, the code for resisting fingerprinting in those system-dependent media queries was useless.

For example nothing would match if you did:

  matchMedia("(-moz-default-windows-theme)").matches;

When the pref was on (which is great, and is the purpose of that code in nsMediaFeatures.cpp!), but there was a super-easy path to get the same result:

<style>
div:-moz-system-metric(default-windows-theme) { color: red; }
</style>
<div></div>
<script>
  const isWindowsDefaultTheme = getComputedStyle(document.querySelector('div')).color == "rgb(255, 0, 0)";
</script>

Bug 1405311 removed that pseudo-class, but made chrome code use the media queries, which respect the pref even for internal use.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #7)
> (In reply to Slew from comment #6)
> > Does this mean that by fixing this bug the resist fingerprinting feature is
> > slightly compromised?
> 
> No, nobody except chrome code can access those media queries anymore.

True and false.

By fixing this in the approach of the proposed patch, it may compromise the resist fingerprinting feature, but it doesn't make anything worse than before, because of what described in comment 4 and comment 8.

The issue here is that, by using the media in chrome sheet inside XBL or UA sheet, you may still be leaking the information to the content document. For example, I suspect content can observe "-moz-windows-classic" via drawing various form controls on canvas using SVG foreignObject. [1]

In this specific case, scrollbar settings is probably not observable via canvas (because we don't create scrollbar at all in SVG-as-image), but it may still be somehow observable via creating a small element and observe its scrollWidth. For example, the following testcase:
> <div style="width: 100px; height: 50px; overflow-y: scroll;"></div>
> <script>alert(document.querySelector('div').scrollWidth);</script>
it shows "83" when the scrollbar only has two buttons (the default), but shows "100" when the scrollbar has four buttons (which you can test via setting privacy.resistFingerprinting to true on current Nightly because of this bug). Similarly "-moz-overlay-scrollbars" would be detectable in the same way.

To be clear, I'm not too concerned about this particular case, especially given that there is nothing worse than before. But if we really want to make resist fingerprinting saner, we should probably return a reasonable default value in GetSystemMetric when resisting fingerprinting, rather than simply skip the check for chrome/UA-only media features.

That said, I'm fine with the proposed patch, so I can r+ it. You can either file a new bug for the fingerprinting, or do something different here.


[1] https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Drawing_DOM_objects_into_a_canvas
Comment on attachment 8920129 [details]
Bug 1408702: Don't honor privacy.resistFingerprinting for media features that aren't accessible to content pages.

https://reviewboard.mozilla.org/r/191120/#review196632
Attachment #8920129 - Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cd7129d6dcd8
Don't honor privacy.resistFingerprinting for media features that aren't accessible to content pages. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/cd7129d6dcd8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Blocks: 1412814
I filed bug 1412814 to track comment 9.
You need to log in before you can comment on or make changes to this bug.