1871516 - fetch().catch: "Promise rejection value is a non-unwrappable cross-compartment wrapper." in MV3 content script

Status: NEW

(WebExtensions :: General, defect, P2)

Description

[Tamo]

Attachment: simple extension

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

Steps to reproduce:

Write an extension which does
fetch(url).then((r)=>{
throw new Error("something");
}).catch((e)=>{
console.error(e.message);
});

Actual results:

"Promise rejection value is a non-unwrappable cross-compartment wrapper."

Expected results:

Just "something"

Maybe you don't want to expose Error.message by design. If so, please close this report.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alex Cornestean]

Hello,

I reproduced the issue on the latest Release (121.0/20231211174248), Beta (122.0b3/20231222091904) and Nightly (123.0a1/20231227205835) under Windows 10 x64 and Ubuntu 22.04 LTS.

With the extensions loaded via about:debugging, accessing a random website (for example, https://ruffle.rs/) and allowing the add-on on the website, will log 2 errors in the web console - Promise rejection value is a non-unwrappable cross-compartment wrapper. and Error: something each time I move the cursor from the console pane to the page content.

For more details, see the attached screenshot.

From my understanding, only the “something” error should be displayed and as such I’ll set the issue to New.

Comment 3

[Alex Cornestean]

Attachment: 2023-12-28_12h42_38.png

Comment 4

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

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

For more information, please visit BugBot documentation.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

The issue described in comment 0 is technically expected behavior, the way webpage content and content scripts (which have more privileged than the webpage they are attached too) are isolated form each other is different in Firefox (a.k.a. "Xray vision") vs WebKit-originated browsers (a.k.a. "Isolated Worlds", for WebKit-based I also mean Chromium-based ones, Chrome technically forked out from WebKit some years ago but should still largely shares the way "isolated worlds" works)

Some more details about Firefox's Xray vision in the context of content scripts works, what to take into account and how to handle a few common cases, is described in this doc page on MDN:

• https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts

In this particular case:

• new Error(...) is going to create an error instance that belongs to the content script sandbox, but the method is attached as an event listener of the webpage and so when it gets called the call will originate from the less privileged webpage principal, and so that is then going to trigger the error "Promise rejection value is a non-unwrappable cross-compartment wrapper"

• replacing that with new window.Error(...) would instead create an error object that belongs to the webpage and so that would violate the sandbox isolation and would just be propagated as the Error instance created, technicaly this error instance would be readable from the webpage content if the webpage gets access to it, and so it should be only used if the Error object is meant to be accessible from the webpage and/or completely safe because it doesn't potentially leak to the webpage details that the webpage should not get access to (in this particular case the string doesn't leak any details, and for error instantiated using window.Error from a content script function the error's filename property is going to be empty and so it doesn't leak the extensions uuid neither)

In general content scripts should be careful to don't blindly create a webcontent accessible object from data or errors objects that belong to the content scripts, as the warning box at the top of the https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts MDN doc page also states.

Comment 6

[Rob Wu [:robwu]]

In Manifest V2 extensions, the fetch global in content script is associated with the content script sandbox, and the code from comment 0 works as expected.

In bug 1578405, we we changed the implementation for Manifest V3, to wrap the fetch API from the web page (without any special extension-specific privileges), which results in the behavior observed in this bug, and explained in comment 5.

There is unfortunately no easy resolution for this class of bugs. Another example in this bug class are linked from bug 1810576, where the streams API are defective due to the security compartments / Xrays.

I suppose that another issue with the current implementation of fetch in MV3 content scripts is that the wrapped fetch won't be able to read other objects instantiated from content scripts, such as Blob instances.

Comment 7

[Rob Wu [:robwu]]

Upon taking a closer look, it seems that this issue is limited to rejections only. That's not great, but not as bad as fetch being broken entirely.

The cause of this bug is that the error message is redacted because the Promise is from the web page (fetch is from the web page in MV3), while the Error instance is created from the content script, which is higher-privileged. Therefore the Promise internals replace the error with a generic error at https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/js/src/builtin/Promise.cpp#1920-1946

The minimal test case, independently of fetch, is as follows:

// execute this in a content script (MV2 or MV3, does not matter):

window.Promise.reject(new Error("huh")).catch(v => console.log(v.message));
// ^ logs: "Promise rejection value is a non-unwrappable cross-compartment wrapper."

window.Promise.resolve(new Error("huh")).then(v => console.log(v.message));
// ^ logs: "huh"

The fact that the resolved value works, but the rejected value does not is a signal that the rejection handling is too strict. At the very least, if the reason is in the same compartment as the cross-compartment wrapper, then the reason should be forwarded without modification. In the worst case, if the Promise implementation were to dereference members of reason (from a higher-privileged compartment), then an error message could be synthesized at that point.

P.S. With the cause known, I would prefer to resolve this bug by fixing the Promise internals, instead of fetch-specific work-arounds. Fixing the Promise internals resolves the issue for much more than just fetch. Additionally, solutions in the direction of "re-introduce a sandbox-specific fetch" should be avoided, because that would result in new regressions (I explained how MV3 fetch resolves a bug in https://bugzilla.mozilla.org/show_bug.cgi?id=1763240#c16). Minimizing the number of sandbox-specific APIs reduces the amount of developer-unfriendly Xray wrapper situations in the wild, because such issues mainly occur whenever someone tries to mix sandbox objects with Xray-wrapped API methods.

Comment 8

[Rob Wu [:robwu]]

The concrete path forwards is to explore a way to avoid redacting the error object in Promise.cpp as explained in comment 7. If there is still any trace of the original content script sandbox at that point, doing so would be easy. Otherwise it is going to be much more involved.

Comment 9

[Stig Nygaard]

I don't understand much/any of technical details above, but I think an issue I have is related?:
https://discourse.mozilla.org/t/firefox-internalerror-mv3/130261