1713619 - PDF thumbnails not working when privacy.resistFingerprinting enabled

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Serge]

Attachment: Screenshot of PDF viewer sidebar showing hatched thumbnails

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

1. In about:preferences, under Applications, set action for PDF documents to "Open in Firefox".
2. Enable privacy.resistFingerprinting.
3. Navigate to a URL that returns a PDF document that can be displayed by Firefox. For example, navigate to https://web.pa.msu.edu/people/duxbury/courses/phy480/Cpp_refcard.pdf.
4. Firefox may display a 'Allow pdf.js t use your HTML5 canvas image data?' dialog near the address bar. If displayed, ignore the prompt or click on [X] 'Close this message' in top-right corner of the dialog.
5. If Sidebar is not visible, click on Toggle Sidebar button to display the sidebar.

Actual results:

Sidebar displays thumbnails of PDF document pages. Each thumbnail is a white rectangle hatched with a different colour. Thumbnails are not actual representations of the PDF pages.

Work-around:

1. Allow HTML5 canvas for the specified URL.
2. Refresh page.

Expected results:

Each thumbnail in the sidebar should be a faithful representation of the actual PDF document page, regardless of privacy.resistFingerprinting setting.
Firefox internal PDF viewer should not need to ask for canvas permissions.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::PDF Viewer' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Tom Ritter [:tjr]]

Yes; we should be able to allow these thumbnails through (I'm assuming that pdf JS can't read DOM data.)

Comment 3

[sanketh]

It looks like this was discussed ~5 years ago on tor trac and on the pdf.js github, the conclusion then seems to be giving pdf.js an exemption from the canvas routines. My quick guess is that pdf.js's recent move to being an addon from an extension broke those special privileges and they need to be reinstated.

Comment 4

[Calixte Denizet (:calixte)]

Could find a regression range in using mozregression:
https://mozilla.github.io/mozregression/

Comment 5

[Alice0775 White]

Regression window:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=196dadb2fe500e75c6fbddcac78106648676cf10&tochange=684b9ee0468e6560b00c69231adfe1b7e68d58a4

Suspect: Bug 967895

Comment 6

[Jonas Jenwald [:Snuffleupagus]]

While I really cannot claim to understand the following code https://searchfox.org/mozilla-central/rev/489e82dcc1e5afbe691ff3b1c982382914637e38/dom/canvas/CanvasUtils.cpp#82-87, I nonetheless cannot help thinking that the particular file-check used there isn't correct.

Note that the (thumbnail) canvases are not being created in that particular file, and the following search suggests that most call-sites (be it in C++ or JS code) are checking the principal or origin for "resource://pdf.js/web/viewer.html" respectively "resource://pdf.js".

Maybe the code in CanvasUtils.cpp could simply utilize this existing helper: https://searchfox.org/mozilla-central/rev/489e82dcc1e5afbe691ff3b1c982382914637e38/dom/base/nsContentUtils.cpp#6717-6725, unless I'm completely misunderstanding things?

Comment 7

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

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Tom Schuster (MoCo)]

Attachment: Bug 1713619 - Exempt chrome/resource (incl PDF.js) principals from canvas randomization/placeholders. r?lsalzman

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8cd1fc9da2b4
Exempt chrome/resource (incl PDF.js) principals from canvas randomization/placeholders. r=lsalzman

Comment 10

[Tom Ritter [:tjr]]

Why do we need this? What's wrong in my assumption in Bug 1830629 that our current exemption for resource:// isn't sufficient?

Comment 11

[Tom Schuster (MoCo)]

Because when we are in Document::RecomputeResistFingerprinting we are using the mChannel, which in this case has the principal/uri of e.g. https://web.pa.msu.edu/people/duxbury/courses/phy480/Cpp_refcard.pdf.

The exemption uses the SubjectPrincipal, which will be the actual principal of the running pdf.js code.

Comment 12

[Tom Ritter [:tjr]]

Okay, gotcha, thanks

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8cd1fc9da2b4

Comment 14

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

The patch landed in nightly and beta is affected.
:tschuster, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[Tom Schuster (MoCo)]

Long standing minor bug in a non-default config => wontfix.

Comment 16

[Tom Ritter [:tjr]]

[Tracking Requested - why for this release]: Would it be possible to backport this to ESR for Tor?

Comment 17

[Tom Schuster (MoCo)]

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

Would it be possible to backport this to ESR for Tor?

We would need a new patch for ESR. Really should have a test for this too.

Comment 18

[Tom Schuster (MoCo)]

Attachment: Bug 1713619 - ESR 115: Exempt chrome/resource principals from canvas randomization/placeholders. r?lsalzman

Comment 19

[Tom Schuster (MoCo)]

Comment on attachment 9370871 [details]
Bug 1713619 - ESR 115: Exempt chrome/resource principals from canvas randomization/placeholders. r?lsalzman

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a longstanding issue with privacy.resistFingerprinting, that we want to fix (mostly) for TOR.
• User impact if declined: PDF.js sidebar is unusable in certain configurations.
• Fix Landed on Version: 123
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Manually verified on ESR and verified on Nightly. Non-default config impacted.

Comment 20

[Tom Schuster (MoCo)]

Attachment: Bug 1713619 - Test canvas placeholder exemption for pdf.js. r?tjr

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9b705d69a04
Test canvas placeholder exemption for pdf.js. r=tjr

Comment 22

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06986c8d3305
Backed out changeset c9b705d69a04 for causing failures at PerformanceMainThread.cpp.

Comment 23

[Phabricator Automation]

Comment on attachment 9371275 [details]
Bug 1713619 - Test canvas placeholder exemption for pdf.js. r?tjr

Revision D197779 was moved to bug 1873405. Setting attachment 9371275 [details] to obsolete.

Comment 24

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/8945a4850ae5

Comment 25

[Pascal Chevrel:pascalc]

Comment on attachment 9370871 [details]
Bug 1713619 - ESR 115: Exempt chrome/resource principals from canvas randomization/placeholders. r?lsalzman

Approved for ESR