Closed Bug 1918257 Opened 1 year ago Closed 6 months ago

bool nsContentUtils::IsPDFJS(nsIPrincipal* aPrincipal) is wrong

Categories

(Firefox :: PDF Viewer, defect, P3)

Product:
Firefox
Component:
PDF Viewer
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
149 Branch
Tracking Flags:
Tracking Status
firefox149 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

Details

Attachments

(1 file)

Bug 1918257: Correctly detect and exempt PDFjs from RFP r?timhuang,tschuster
48 bytes, text/x-phabricator-request
Details | Review

I'm not sure if this should be DOM or pdfjs.

We have a function IsPDFJS and a related function IsSystemOrPDFJS and they check to see if the principal's spec is "resource://pdf.js/web/viewer.html".

Which it isn't. Neither the Document's NodePrincipal, nor the LoadInfo's LoadingPrincipal or TriggeringPrincipal are ever that value for the two mst common scenarios I tested: opening a pdf in a new tab, and embedding a pdf with an iframe.

In a new tab, the NodePrincipal is a null principal. The LoadInfo's LoadingPrincipal is NULL - which is the exception to the rule that all TYPE_DOCUMENT's have a non-null LoadingPrincipal - and the triggering principal is System.

In the embed case, it is a TYPE_SUBDOCUMENT, and the loadingprincipal and triggering principal are the embedding document.

This is problematic for a few reasons, one is that all the existing checks will be false in cases we may want them to be true but also because there is no working way to test for pdf.js that is supported and future-proof.

Now, the weird thing is, code coverage does show a very small number of times the function is actually true.

I do not know what makes it true these times, I couldn't find a way to cause it myself.

I don't know how hot this function is, but if there are going to be changes to pdfjs's principal, creating a principal here https://searchfox.org/mozilla-central/rev/14a66905574728bc9eb60e927d17d98721167dcf/dom/base/nsContentUtils.cpp#824-826 and checking it could be faster than string comparison or that's what I've been told. Just leaving it here in case assignee finds it useful

I'd be inclined to say that's a DOM bug more than a pdf.js one.
On the js side, we use browser.contentPrincipal:
https://searchfox.org/mozilla-central/search?q=resource%3A%2F%2Fpdf.js%2Fweb%2Fviewer.html&path=mjs&case=false&regexp=false

and as far as I can tell it works correctly.
If you've an example where the contentPrincipal isn't pdfjs when it should, please tell me.

I think the principal is set there:
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#1259-1269

It's a bit strange, because it should have worked correctly in the past... (at least I suppose it should have).

The bit in PdfStreamConverter.sys.mjs looks like we're creating a contentprincipal of the PDF file URL, which is likely better than using the SystemPrincipal.
For fetching the PDF itself, I think we could create a contentpolicytype and make it TYPE_PDF_DOCUMENT? If we need to detect the pdf viewer, then we should be able to use the resource:// URL.

@Tom: Do you need the checks for the PDF viewer or the document inside of the PDF viewer?

(In reply to Frederik Braun [:freddy] from comment #3)

The bit in PdfStreamConverter.sys.mjs looks like we're creating a contentprincipal of the PDF file URL, which is likely better than using the SystemPrincipal.
For fetching the PDF itself, I think we could create a contentpolicytype and make it TYPE_PDF_DOCUMENT? If we need to detect the pdf viewer, then we should be able to use the resource:// URL.

@Tom: Do you need the checks for the PDF viewer or the document inside of the PDF viewer?

We want to know when a document is hosted in PdfJS so we can disable fingerprinting protections for it and make it more readable/accessible to users. That also seems to be the theme of all of the existing uses above that are probably broken. I think any reliable method to do this would work - we don't have to modify the NodePrincipal just because that's what we seemed to do in the past.

I actually think that it is preferable to have the NodePrincipal of a pdf document represent the real origin so we don't accidently upgrade or over-privilege it. (The risk of doing so is limited, but non-zero because pdf has limited scripting support and I could imagine an exploit for the JS engine being crafted and used if it were valuable to do so because pdfs were accidently or on-purpose higher-privileged than intended.)

The embedded JS in a pdf, is executed in a sandbox with a null principal:
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/content/PdfSandbox.sys.mjs#39
so it should be safe whatever the privileges the pdf has.

Now, the weird thing is, code coverage does show a very small number of times the function is actually true.

That said we don't have that much tests in m-c (there are mostly in the github repo):
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test

We don't open that much pdfs during the tests, so I'm not sure the numbers are meaningful to guess that something is wrong with isPDFJS or the principal used for the check.

I don't know why the code coverage reports 0 here:
https://coverage.moz.tools/#revision=latest&path=dom/base/Element.cpp&view=file&line=3644

because it should be tested by:
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test/browser_pdfjs_fullscreen.js

Out of curiosity, I replaced the return nullptr by return "FullscreenDeniedMouseEventOnlyLeftBtn" and the test is now failing with this error:

Console message: [JavaScript Warning: "Request for fullscreen was denied because Element.requestFullscreen() was called from inside a mouse event handler not triggered by left mouse button." {file: "resource://pdf.js/web/viewer.mjs" line: 5727}]

And trying to switch to presentation mode in using ctrl+shift+p from the pdf viewer isn't working.

So I'm really not sure that IsPDFJS or the principal used in it are so wrong at least if we only base our judgement on the code coverage numbers.

See Also: → 1450443

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

For more information, please visit BugBot documentation.

Flags: needinfo?(cdenizet)
Severity: -- → S3
Flags: needinfo?(cdenizet)
Priority: -- → P3

In a new tab, the NodePrincipal is a null principal. The LoadInfo's LoadingPrincipal is NULL - which is the exception to the rule that all TYPE_DOCUMENT's have a non-null LoadingPrincipal - and the triggering principal is System.

I can't reproduce this, when e.g. testing with this empty pdf. Maybe it depends on when you try to check the principal? In Document::LoadEventFired the document's nodeprincipal is definitely the correct resource://pdf.js one and nsContentUtils::IsPDFJS returns true.

In the parent gBrowser.contentPrincipal.spec also seems correct.

Yes correct, it works when you open a PDF directly, but if it is an embed, then it doesn't work.

Example, run the following on about:blank (or any page with a lax CSP)

const embed = document.createElement("embed");
embed.src = "https://mag.wcoomd.org/uploads/2018/05/blank.pdf";
document.body.appendChild(embed);

It is really weird though because, when I place the following MOZ_LOG here,

  nsCString spec;
  NodePrincipal()->GetSpec(spec);
  MOZ_LOG(myLogModule, LogLevel::Debug,
          ("Inside RecomputeResistFingerprinting with URI %s and principal %s",
           GetDocumentURI() ? GetDocumentURI()->GetSpecOrDefault().get()
                            : "null", spec.get()));

I get the following for directly loaded pdf:

Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{d8aaf693-abc2-4ca5-a440-0326f57d590f}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{d8aaf693-abc2-4ca5-a440-0326f57d590f}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0

and when I load a PDF using an embed I get the following:

Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{e964d7c4-d213-47e9-8095-5cebc57691d8}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{e964d7c4-d213-47e9-8095-5cebc57691d8}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI about:blank and principal moz-nullprincipal:{a716c017-09ee-4911-a0e5-06a9b23e55a9}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI about:blank and principal moz-nullprincipal:{a716c017-09ee-4911-a0e5-06a9b23e55a9}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{ac7b862e-a9c8-4e38-902f-3709840ce411}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{ac7b862e-a9c8-4e38-902f-3709840ce411}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI about:blank and principal moz-nullprincipal:{b4a91db0-53f2-4de4-98b7-b0d568d89b79}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{6a50107c-3fa0-4987-9d8f-602dbe89f76c}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI null and principal moz-nullprincipal:{6a50107c-3fa0-4987-9d8f-602dbe89f76c}
Finished RecomputeResistFingerprinting with result 1
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0
Inside RecomputeResistFingerprinting with URI https://mag.wcoomd.org/uploads/2018/05/blank.pdf and principal resource://pdf.js/web/viewer.html
Finished RecomputeResistFingerprinting with result 0

Looking at last three Inside RecomputeResistFingerprinting lines, one would think it should work, but it doesn't :/

Edit: Here's the CSM Log of loading pdf in embed on about:blank. Unfortunately checking for loading principal doesn't seem to work either

[Parent 60367: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck Begin
[Parent 60367: Main Thread]: V/CSMLog doContentSecurityCheck:
[Parent 60367: Main Thread]: V/CSMLog   processType: ""
[Parent 60367: Main Thread]: V/CSMLog   channelURI: "https://mag.wcoomd.org/uploads/2018/05/blank.pdf"
[Parent 60367: Main Thread]: V/CSMLog   httpMethod: GET
[Parent 60367: Main Thread]: D/CSMLog   loadingPrincipal: NullPrincipal
[Parent 60367: Main Thread]: D/CSMLog   triggeringPrincipal: NullPrincipal
[Parent 60367: Main Thread]: D/CSMLog   principalToInherit: NullPrincipal
[Parent 60367: Main Thread]: V/CSMLog   redirectChain:
[Parent 60367: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_INTERNAL_EMBED
[Parent 60367: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_OBJECT
[Parent 60367: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Parent 60367: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Parent 60367: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: false
[Parent 60367: Main Thread]: V/CSMLog   schemelessInput: 0
[Parent 60367: Main Thread]: D/CSMLog   CSP:
[Parent 60367: Main Thread]: V/CSMLog   securityFlags:
[Parent 60367: Main Thread]: V/CSMLog     - SEC_ALLOW_CROSS_ORIGIN_SEC_CONTEXT_IS_NULL
[Parent 60367: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Parent 60367: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Parent 60367: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck End
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck Begin
[Child 60391: Main Thread]: V/CSMLog doContentSecurityCheck:
[Child 60391: Main Thread]: V/CSMLog   processType: "webIsolated=https://wcoomd.org"
[Child 60391: Main Thread]: V/CSMLog   channelURI: "https://mag.wcoomd.org/uploads/2018/05/blank.pdf"
[Child 60391: Main Thread]: D/CSMLog   loadingPrincipal: SystemPrincipal
[Child 60391: Main Thread]: D/CSMLog   triggeringPrincipal: SystemPrincipal
[Child 60391: Main Thread]: D/CSMLog   principalToInherit: nullptr
[Child 60391: Main Thread]: V/CSMLog   redirectChain:
[Child 60391: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_OTHER
[Child 60391: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_OTHER
[Child 60391: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Child 60391: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Child 60391: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: true
[Child 60391: Main Thread]: V/CSMLog   schemelessInput: 0
[Child 60391: Main Thread]: D/CSMLog   CSP:
[Child 60391: Main Thread]: V/CSMLog   securityFlags:
[Child 60391: Main Thread]: V/CSMLog     - SEC_ALLOW_CROSS_ORIGIN_SEC_CONTEXT_IS_NULL
[Child 60391: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Child 60391: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck End
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck Begin
[Child 60391: Main Thread]: V/CSMLog doContentSecurityCheck:
[Child 60391: Main Thread]: V/CSMLog   processType: "webIsolated=https://wcoomd.org"
[Child 60391: Main Thread]: V/CSMLog   channelURI: "resource://pdf.js/build/pdf.mjs"
[Child 60391: Main Thread]: D/CSMLog   loadingPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   triggeringPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   principalToInherit: nullptr
[Child 60391: Main Thread]: V/CSMLog   redirectChain:
[Child 60391: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_INTERNAL_MODULE_PRELOAD
[Child 60391: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_SCRIPT
[Child 60391: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Child 60391: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Child 60391: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: false
[Child 60391: Main Thread]: V/CSMLog   schemelessInput: 0
[Child 60391: Main Thread]: D/CSMLog   CSP:
[Child 60391: Main Thread]: V/CSMLog   securityFlags:
[Child 60391: Main Thread]: V/CSMLog     - SEC_REQUIRE_CORS_INHERITS_SEC_CONTEXT
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_SAME_ORIGIN
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_OMIT
[Child 60391: Main Thread]: V/CSMLog     - SEC_ALLOW_CHROME
[Child 60391: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Child 60391: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck End
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck Begin
[Child 60391: Main Thread]: V/CSMLog doContentSecurityCheck:
[Child 60391: Main Thread]: V/CSMLog   processType: "webIsolated=https://wcoomd.org"
[Child 60391: Main Thread]: V/CSMLog   channelURI: "resource://pdf.js/web/viewer.mjs"
[Child 60391: Main Thread]: D/CSMLog   loadingPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   triggeringPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   principalToInherit: nullptr
[Child 60391: Main Thread]: V/CSMLog   redirectChain:
[Child 60391: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_INTERNAL_MODULE_PRELOAD
[Child 60391: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_SCRIPT
[Child 60391: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Child 60391: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Child 60391: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: false
[Child 60391: Main Thread]: V/CSMLog   schemelessInput: 0
[Child 60391: Main Thread]: D/CSMLog   CSP:
[Child 60391: Main Thread]: V/CSMLog   securityFlags:
[Child 60391: Main Thread]: V/CSMLog     - SEC_REQUIRE_CORS_INHERITS_SEC_CONTEXT
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_SAME_ORIGIN
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_OMIT
[Child 60391: Main Thread]: V/CSMLog     - SEC_ALLOW_CHROME
[Child 60391: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Child 60391: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck End
console.warn: "Listener for event 'frame' did not return a promise."
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck Begin
[Child 60391: Main Thread]: V/CSMLog doContentSecurityCheck:
[Child 60391: Main Thread]: V/CSMLog   processType: "webIsolated=https://wcoomd.org"
[Child 60391: Main Thread]: V/CSMLog   channelURI: "resource://pdf.js/build/pdf.worker.mjs"
[Child 60391: Main Thread]: D/CSMLog   loadingPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   triggeringPrincipal: "resource://pdf.js/web/viewer.html"
[Child 60391: Main Thread]: D/CSMLog   principalToInherit: nullptr
[Child 60391: Main Thread]: V/CSMLog   redirectChain:
[Child 60391: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_INTERNAL_WORKER
[Child 60391: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_SCRIPT
[Child 60391: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Child 60391: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Child 60391: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: false
[Child 60391: Main Thread]: V/CSMLog   schemelessInput: 0
[Child 60391: Main Thread]: D/CSMLog   CSP:
[Child 60391: Main Thread]: V/CSMLog   securityFlags:
[Child 60391: Main Thread]: V/CSMLog     - SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_SAME_ORIGIN
[Child 60391: Main Thread]: V/CSMLog     - SEC_COOKIES_OMIT
[Child 60391: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Child 60391: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Child 60391: Main Thread]: D/CSMLog 
#DebugDoContentSecurityCheck End

Alright, resolving this bug is now a blocker to Bug 1990514 . I had claude look at it, and we came up with the below. It sounds alright to me, but I'd like some more review on it.

In particular, I'm not 100% sure that the _dangerous functions don't need to be able to detect PDF.js. What about downloading a pdf? CookieJar creation?

Bug 1918257 Investigation: IsPDFJS and RFP Exemption

Disagreements with Bug Comments Based on Empirical Testing

Comment #0 (the original report from 2024) states:

Neither the Document's NodePrincipal, nor the LoadInfo's LoadingPrincipal or
TriggeringPrincipal are ever that value for the two most common scenarios I
tested: opening a pdf in a new tab, and embedding a pdf with an iframe.

In a new tab, the NodePrincipal is a null principal.

This is no longer the case. Using fprintf-based debugging in
Document::RecomputeResistFingerprinting, we confirmed that in the current
codebase (February 2026), the Document's NodePrincipal is
resource://pdf.js/web/viewer.html for both direct and embedded PDF loads.
nsContentUtils::IsPDFJS(NodePrincipal()) returns true in both cases.

Comment #10 by mozbugzilla@fklc.dev actually observed this same thing in
December 2024 -- the last three log entries in their embedded PDF trace show
principal resource://pdf.js/web/viewer.html -- but noted confusion that
"one would think it should work, but it doesn't." Our investigation explains
why: the principal is correct, but nothing in the RFP decision path checks it.

Comment #8 by tschuster@mozilla.com (December 2024) also confirmed that the
NodePrincipal is correct when checked at Document::LoadEventFired time, and
that IsPDFJS returns true. This is consistent with our findings.

The original report's observation of a null principal was likely either from an
earlier version of the code, or from checking the principal at a different
point in the document lifecycle (e.g., before PdfStreamConverter sets it).

What Print Debugging Revealed

We added fprintf statements to Document::RecomputeResistFingerprinting that
logged the document URI, NodePrincipal, parent document info, channel
loadinfo principals, content policy type, and IsPDFJS result.

Direct PDF load (e.g. https://ritter.vg/p/tor-v1.6.pdf)

URI=https://ritter.vg/p/tor-v1.6.pdf
NodePrincipal=resource://pdf.js/web/viewer.html
isPDFJS=1
parentURI=none
hasChannel=1
loadingPrincipal=NULL
triggeringPrincipal=(system)
contentType=6 (TYPE_DOCUMENT)
RESULT: mShouldRFP=1

Embedded PDF load (iframe on https://ritter.vg/misc/ff/embed-pdf.html)

URI=https://ritter.vg/p/tor-v1.6.pdf
NodePrincipal=resource://pdf.js/web/viewer.html
isPDFJS=1
parentURI=https://ritter.vg/misc/ff/embed-pdf.html
parentPrincipal=https://ritter.vg/misc/ff/embed-pdf.html
hasChannel=1
loadingPrincipal=https://ritter.vg/misc/ff/embed-pdf.html
triggeringPrincipal=https://ritter.vg/misc/ff/embed-pdf.html
contentType=29 (TYPE_SUBDOCUMENT)
RESULT: mShouldRFP=1

In both cases, IsPDFJS is true but the final result is mShouldRFP=1
(fingerprinting protection applied). The root cause: RecomputeResistFingerprinting
takes the mChannel branch and calls nsContentUtils::ShouldResistFingerprinting(mChannel),
which extracts the channel URI (https://ritter.vg/p/tor-v1.6.pdf) for
TYPE_DOCUMENT/TYPE_SUBDOCUMENT loads. That https:// URL does not trigger the
resource:// scheme exemption in SchemeSaysShouldNotResistFingerprinting,
and the domain is not on the exempt list, so RFP is applied.

The NodePrincipal (resource://pdf.js/web/viewer.html) is never consulted
anywhere in the RFP decision chain.

The Fix

We added a check in nsContentUtils::ShouldResistFingerprinting(nsIChannel*)
that retrieves the channel's result principal via
GetChannelResultPrincipal() and checks IsPDFJS() on it. The result
principal is the principal the document will end up with -- for PDF.js
documents, this is resource://pdf.js/web/viewer.html. The check is placed
after the positive return check (so we don't bother if RFP prefs aren't even
enabled) and before the ETP/CookieJarSettings/content-type branching.

Why This Is the Correct Place

The nsContentUtils::ShouldResistFingerprinting(nsIChannel*) overload is the
function that Document::RecomputeResistFingerprinting calls to compute the
cached mShouldResistFingerprinting value for the document. This cached value
is then returned by Document::ShouldResistFingerprinting(), which in turn
is what all the other ShouldRFP overloads (the Document, DocShell, and
GlobalObject variants) ultimately delegate to. By fixing the channel overload,
the cached value is computed correctly, and every downstream consumer
automatically gets the right answer.

Placing the check here also means it applies at channel setup time, before the
document is fully constructed. This is important because some RFP behaviors
may take effect during document loading. If we only checked later (e.g., in
Document::ShouldResistFingerprinting itself), there could be a window where
the document is being set up with RFP applied before the exemption kicks in.

Why the check is not needed in ChromeUtils::ShouldResistFingerprinting

ChromeUtils::ShouldResistFingerprinting is a JS-accessible API used by
browser chrome code (browser-init.js, BrowserGlue, SessionStore, RFPHelper,
GeckoViewSettings, etc.) for decisions like window sizing, color scheme,
locale, and user agent. It operates purely on the window's private browsing
mode and the RFP prefs/target -- it has no document or principal context at
all. PDF.js does not call this function, and the chrome code that does call it
is making global browser-level decisions (e.g., "should we round the window
size?") that are not specific to any content document. Adding an IsPDFJS
check here would require threading document context through an API that is
intentionally document-agnostic, and there is no current need for it.

Why the check is not needed in the _dangerous functions

The two ShouldResistFingerprinting_dangerous terminal functions (the
nsIURI* overload and the nsIPrincipal* overload) are called from two
places: internally from ShouldRFP(nsIChannel*), and externally from
about a dozen call sites in service worker setup, worker creation, LoadInfo
construction, CookieJarSettings creation, and WebCodecs. None of the external
callers would ever encounter a PDF.js principal -- they deal with service
worker principals, loading principals of add-on injected resources, download
channel principals, and JS realm principals. The only calls where PDF.js is
relevant are the two internal calls from ShouldRFP(nsIChannel*), and our
fix intercepts those before they reach the _dangerous functions. Adding
IsPDFJS checks to the _dangerous functions would be redundant and would
add overhead to many unrelated code paths.

Furthermore, the URI-based _dangerous function already contains a
SchemeSaysShouldNotResistFingerprinting(aURI) check that exempts
resource:// URLs -- the problem was never that the exemption logic was
missing, but that the wrong URI (the PDF content URL rather than the viewer
URL) was being passed to it. Our fix addresses the problem at the point where
the wrong value would be selected, rather than trying to compensate for it
downstream.

Test

A browser chrome test (browser_pdfjs_rfp_exemption.js) was added that:

  1. Enables privacy.resistFingerprinting, loads a PDF, and verifies that
    browsingContext.currentWindowContext.shouldResistFingerprinting is false
    (PDF.js is exempted).
  2. Enables privacy.resistFingerprinting, loads a normal web page, and
    verifies that shouldResistFingerprinting is true (normal pages are still
    protected).

The test was confirmed to fail without the fix and pass with it.

Blocks: 1990514
Assignee: nobody → tom
Status: NEW → ASSIGNED

cc'ing piero for Tor Browser if he wants to backport :)

(In reply to Thorin [:thorin] from comment #13)

cc'ing piero for Tor Browser if he wants to backport :)

Thanks!
Maybe we could ask for uplift once this lands and is tested in nightly 😄.

As a heads up, I'm landing this in Nightly - as you saw in Bug 1990514 - pdfjs was not exempted from any fingerprinting protections that might be enabled. When we enabled them in normal browsing mode, they broke things. They are almost certainly broke in the same way in PBM today. This patch should exempt pdfjs from all protections, so PBM will be unbroke and when we land 1990514 it will also not break things. But this patch is changing behaviors (especially in PBM today) so I wanted to give you a heads up about it in case you see anything unusual.

Flags: needinfo?(cdenizet)
Regressions: 2017243

This should fix Bug 1746679 and any related...

Blocks: 1746679

:tom, when I see the number of messages in the devtools console, I've the same wondering as in https://bugzilla.mozilla.org/show_bug.cgi?id=1918257#c1. Wdyt ?

Flags: needinfo?(cdenizet) → needinfo?(tom)

should also fix Bug 1917618

https://hg.mozilla.org/mozilla-central/rev/5d22711646b9

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
status-firefox149: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 149 Branch
Regressions: 2022084
Flags: needinfo?(tom)
Blocks: 1917618
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: