Closed Bug 1984003 Opened 1 year ago Closed 1 year ago

passff can no longer find or fill out login fields in nightly

Categories

(WebExtensions :: General, defect)

defect

Tracking

(firefox-esr128 unaffected, firefox-esr140 unaffected, firefox142 unaffected, firefox143 fixed, firefox144 fixed)

RESOLVED DUPLICATE of bug 1985652
Tracking Status
firefox-esr128 --- unaffected
firefox-esr140 --- unaffected
firefox142 --- unaffected
firefox143 --- fixed
firefox144 --- fixed

People

(Reporter: shtrom, Assigned: dholbert)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached image passff-expected.png —

When using the PassFF extension to link to the pass local password manager, with recent versions of Nightly in no longer manages to draw it's widget or fill-out form inputs.

First broken build was Nigthly 2025-08-13, at commit 476f23ad, for bug 1970490.

Bug 1970490: Use loading principal (rather than triggering principal) for CORS checks, by default. r=smaug

This is essentially a backout of bug 1496505, putting its change behind a new
off-by-default about:config pref[1] for now, in case there are use cases that
require it.

[1] content.cors.use_triggering_principal

Differential Revision: https://phabricator.services.mozilla.com/D252611

Reproduction steps

  1. Install https://addons.mozilla.org/en-US/firefox/addon/passff/?utm_source=addons.mozilla.org&utm_medium=referral&utm_content=search
  2. Go to a page with a login form, say https://auth.wikimedia.org/enwiki/wiki/Special:UserLogin
  3. Observe the login input field.

Expected: there should be a little p widget on the right-hand-side inside the input field, allowing to interact with the password manager.

Actual after 476f23ad: no widget in the input form.

Attached image passff-actual.png —
Summary: passff can no longer find or fill out login fields → passff can no longer find or fill out login fields in nightly
Keywords: regression
Regressed by: CVE-2025-10529

Security Error: Content at https://auth.wikimedia.org/enwiki/wiki/Special:UserLogin may not load or link to moz-extension://9929e7f1-94c6-4ea9-84f6-75de17742811/modules/main.js.

What happens is that this extension has a "loader" content script content_script.js that just does import(browser.runtime.getURL("modules/main.js"));. I am a bit surprised that this was broken, I explicitly added support for dynamic imports in content scripts in bug 1536094 with a test.

(In reply to Tom Schuster (MoCo) from comment #3)

Security Error: Content at https://auth.wikimedia.org/enwiki/wiki/Special:UserLogin may not load or link to moz-extension://9929e7f1-94c6-4ea9-84f6-75de17742811/modules/main.js.

What happens is that this extension has a "loader" content script content_script.js that just does import(browser.runtime.getURL("modules/main.js"));. I am a bit surprised that this was broken, I explicitly added support for dynamic imports in content scripts in bug 1536094 with a test.

Shot in the dark... maybe this has something to do with bug 1803950 comment 4? (i.e. maybe the resource in question in this particular bug isn't "web-accessible" i.e. listed in web_accessible_resources, and the CORS-confusion that I fixed in bug 1970490 was papering over that.)

Looking at the extension's manifest, it doesn't list main.js in web_accessible_resources.

Yup, I can fix this bug (get "Expected" results in comment 0) by just adding main.js and all of the scripts that it loads to "web_accessible_resources" in the extension's manifest here.

So that ends up looking like this (the png and svg files are preexisting):

  "web_accessible_resources": [
    "modules/auth.js",
    "modules/main.js",
    "modules/menu.js",
    "modules/page.js",
    "modules/pass.js",
    "modules/preferences.js",
    "modules/util.js",
    "skin/paper-plane.svg",
    "skin/pencil-square.svg",
    "skin/icon-light.svg",
    "icon.png"
  ],

Tom, do you know if this^ looks like the correct diagnosis/fix here (i.e. fixing this on the add-on's side)? I'm not super familiar with WebExtensions & web_accessible_resources but it looks like maybe the right fix, given bug 1803950 comment 4 and bug 1783078.

Flags: needinfo?(dholbert) → needinfo?(tschuster)

Thanks Daniel. I totally forgot that when started supporting dynamic imports in WebExtensions that those had to be listed in the web_accessible_resources. To be clear: I think it would be much nicer if that wasn't required. The imports should stay internal to the extension and not be exposed to the web.

We did have a test that was supposed to ensure that we only allows imports web_accessible_resources from files in could be loaded: https://searchfox.org/firefox-main/rev/d683b2b2ed86192b3a87150c21fc51ba02a93142/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_module_import.js#90-94. I am still confused as to when we started supporting this, or if it always worked in certain cases. The introducer of bug 1984003 seems to have landed before the initial support of dynamic imports in WebExtensions.

Flags: needinfo?(tschuster)

Can we specifically support dynamic import() without requiring web_accessible_resources? That question came up before in bug 1803950.

See Also: → 1803950

:zombie, pinging as triage owner, could this get a Priority/Severity? Is it something we need to track for Fx143?

Flags: needinfo?(tomica)

Two things we could do here, if we anticipate other extensions might hit this issue as well and might need to mitigate this on our end before 143 releases (rather than by changing the extensions with e.g. patches to them in comment 5):

(1) we can toggle content.cors.use_triggering_principal to true to restore the old behavior (effectively backing out the regressor, bug 1970490)
(2) we could perhaps make the logic in bug 1970490 a bit more subtle, to "bless" extensions as a triggering principal, to give them special permission to get the old behavior (while still preserving the new/correct behavior for web content where there's no extension involvement).

(In reply to Olivier Mehani [:shtrom] from comment #0)

Differential Revision: https://phabricator.services.mozilla.com/D252611

When I want to open this, I need to login. After logging in, I get

Unhandled Exception ("AphrontMalformedRequestException")

Your browser did not submit a "phcid" cookie with client state information in the request. Check that cookies are enabled. If this problem persists, you may need to clear your cookies.

I do now know what this mean, but I cannot see the content behind your URL.

Thanks. That phabricator revision is hidden for most folks because it's associated with a security bug (bug 1970490). It'll likely be opened up in a matter of months, but it's still hidden for now.

But I filed bug 1985625 on the fact that phabricator seems to show a confusing error message for this scenario.

(In reply to Daniel Holbert [:dholbert] from comment #9)

(1) we can toggle content.cors.use_triggering_principal to true to restore the old behavior (effectively backing out the regressor, bug 1970490)

I did set content.cors.use_triggering_principal to true in about:config.

But as I read:

(In reply to Daniel Holbert [:dholbert] from comment #11)

a security bug (bug 1970490).

does this open a security hole? If so, what is the actual danger (is that already public?)?

So I should set content.cors.use_triggering_principal to false again for security reasons and have to wait until the extension is fixed or Firefox has devices another way to allow the extension to work again the "old" way?

Regards!

I did set content.cors.use_triggering_principal to true in about:config.
does this open a security hole? If so, what is the actual danger (is that already public?)?

The details aren't public; it's safer (and also more correct) to leave that pref set to false. Setting it to true creates a potential for one site to learn information about another site that it shouldn't have access to; but the potential is pretty remote (it requires very particular configuration on the site-whose-resources-are-being-improperly-read, which may not actually exist in the real world). So I'm not too worried about leaving it true for another release cycle (which represents the same configuration we've been shipping in for a while).

But in any case, I'm also working on a patch to do option (2) from comment 9 today, which makes the pref-flip unnecessary.

Depends on: 1985652

(patch up in bug 1985652. Self-assigning to reflect that this is being worked on.)

Assignee: nobody → dholbert
Flags: needinfo?(tomica)

(In reply to Donal Meehan [:dmeehan] from comment #8)

:zombie, pinging as triage owner, could this get a Priority/Severity? Is it something we need to track for Fx143?

Not sure about priority/severity but I think we do need to track it, yeah. I'll request tracking on bug 1985652 where I posted the patch.

(Assuming reviewers agree on the strategy over there, I intend to dupe this bug to bug 1985652 when the patch is ready to land; I've confirmed that the patch fixes the extension in question here.)

Thanks for working on this regression. Would you mind including a unit test to avoid this regression in the future? Feel free to ask for pointers or tag me as reviewer if you get stuck.

Sure! (Emilio requested one in review over on bug 1985652 too). I'll post an additional test-patch over there, before landing the fix (or will update the main patch with the test & will get the test reviewed before landing).

(In reply to Tom Schuster (MoCo) from comment #6)

We did have a test that was supposed to ensure that we only allows imports web_accessible_resources from files in could be loaded: https://searchfox.org/firefox-main/rev/d683b2b2ed86192b3a87150c21fc51ba02a93142/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_module_import.js#90-94. I am still confused as to when we started supporting this, or if it always worked in certain cases. The introducer of bug 1984003 seems to have landed before the initial support of dynamic imports in WebExtensions.

Just restating this^ for clarity (since I didn't fully understand the context at first): I think you're saying that you're surprised that Firefox had ever allowed these imports from the PassFF add-on, because this^ test was asserting that they shouldn't be allowed, and the test has been passing, and yet PassFF has been successfully doing these non-web-exposed imports up until my patch in bug 1985652 landed.

Assuming I'm paraphrasing you correctly: I think I can explain the confusion. This test got a different level of strictness than PassFF does, because the test opts-in to stricter behavior, by toggling a pref:
https://searchfox.org/firefox-main/rev/d683b2b2ed86192b3a87150c21fc51ba02a93142/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_module_import.js#73

Services.prefs.setBoolPref("extensions.content_web_accessible.enabled", true);

That pref's documentation says that it "enforces the use of web_accessible_resources for content loads...for MV2"

So the test is opting into a stricter behavior, and then asserting and correctly observing strictness; but without the pref flip, we've been more lax (until bug 1985652 independently made us stricter, which caused the loads in PassFF to stop working in the default configuration).

Status: NEW → RESOLVED
Closed: 1 year ago
Duplicate of bug: 1985652
Resolution: --- → DUPLICATE

This looks fixed in Nightly 2025-08-31. Thanks everyone!

No longer depends on: 1985652
See Also: 1803950
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: