1939087 - Very long add-on name can spoofs and breaks add-ons manager UI

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Farras Givari]

Attachment: borderify.js

Summary & Details

By giving a long name in Firefox Add-Ons, it is possible to spoof all of extensions manager UI and also breaks various parts of the UI. Even without special permissions, long names can cause other UI issues like, we can't even open the add-ons manager pop up (or will takes significantly longer to read, especially on devices with limited hardware resources, such as minimal memory), add-ons manager details page is obscured. So if the user installed the add-ons once, the "poisoned" add-ons will be there forever, especially for users with limited hardware resources, because all of Firefox Add-Ons options will takes significantly longer to read or even can't be open at all.

References: https://issues.chromium.org/issues/40063885

Steps to Reproduce

1. Download the borderify.js and manifest.json that i've attach in Google Drive link
2. Create an icon for the extension
3. Load the extension in Firefox

Note

Google Drive link with borderify.js manifest.json and the video PoC:
https://drive.google.com/drive/folders/1YoZkIu3sVVA8Alhb__t2gDyZnCZ28Ude?usp=sharing

Comment 1

[Rob Wu [:robwu]]

Thanks for your report! The reported bug looks valid, but due to several mitigating factors, the security impact seems to be low.

In the video, the overview of add-on manager UI itself does not look broken. The user can still click on the toggle to disable the add-on.
The details page looks missing, because the long title causes the title to expand to many lines.

AMO restricts the length of the name field to 45 characters, so an add-on such as shown in the test case cannot be published on AMO. This means that the only way to load such an extensions on regular versions of Firefox is manually through about:debugging (or similarly, the devtools protocol). Even if the add-on manager UI were to be broken, such extensions can only be loaded temporarily, and are gone after Firefox restarts.

The ability to rely on AMO's validation depends on the browser requiring the extensions to be signed. There is one add-on type without such requirement, i.e. dictionaries (bug 1753276). Because dictionaries are displayed in a view independent of extensions, this bug affecting dictionaries does not impact the UI of extension management.

P.S.: The manifest.json file in the PoC is 35 MB. I cannot even get it to load in Firefox due to its size. That is independent of the name field, and I'll file a separate bug for that.

Comment 2

[Farras Givari]

Thanks for the update! I agree with you regarding the low security impact of this bug.

Based on my CVSS 3.1 calculation, here are the results:
CVSS:3.1/AV:L/AC:H/PR:H/UI:R/S:C/C:N/I:N/A:L (Low 2.3)

I’ll wait for the next update and the final decision regarding this security bounty report!

Comment 3

[Rob Wu [:robwu]]

After fixing bug 1939488, I was able to load the extension and can confirm that the about:addons UI becomes sluggish when the demo extension is loaded. I'm going to attach a fix for this bug too.

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1939087 - Truncate long name and log warning

Comment 5

[Rob Wu [:robwu]]

In comment 1 I mentioned that the impact does not affect release because AMO enforces a name length of 45, through the linter.
The linter does however not account for localizations yet (https://github.com/mozilla/addons-linter/issues/4910), and therefore it is possible for AMO to accept extensions with longer names.

I scanned all public add-ons on AMO and saw that at the surface, that all manifest.json files adhered to the specified 45 max length. This is because AMO does effectively enforce the maximum length of the value in manifest.json.

After also accounting for localizations, I see that there are 1818 localizations among 286 unique add-ons whose name is longer than 45.

• The longest is 81 (one unique occurrence, an add-on with 75 users).
• The next longest is 78 (two add-ons, 49 and 507 users).
• The next longest is 77 (one add-ons, 82 users)
• The next longest is 75 (5 add-ons: 73256, 47338, 320, 82 and 8 users).

Based on this, I think that we can safely enforce a limit of 75 (up from the original 45), which is consistent with what the Chrome Web Store supports.

Comment 6

[Rob Wu [:robwu]]

Here is a spreadsheet with a full overview of the public add-ons on AMO, by name length.
I color-coded the lengths: <= 50 is green, 51 <= length <= 75 is yellow, >=76 is red.

https://docs.google.com/spreadsheets/d/1Q9mpKO_VrAopJCfuZjXMDD-uIOCoDTBTodVirPIeph0/edit?gid=2100139108#gid=2100139108

Comment 7

[Rob Wu [:robwu]]

I discussed the topic of acceptable name length with my team and product yesterday, and we are going to bump the limit to 75.

On AMO's side the maximum length used to be 70 in a very distant past before it was changed to 50 (https://github.com/mozilla/addons-server/commit/f580e11fbb3). AMO will also change the limit to 75, as part of https://github.com/mozilla/addons/issues/15268

Interesting data point - Chrome used to have a maximum length of 45, but increased that to 75 almost one year ago, announced at https://groups.google.com/a/chromium.org/g/chromium-extensions/c/mpDvFpT0KJM/m/WWFFQZFyAAAJ

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3941628a2766
Truncate long name and log warning r=zombie,willdurand

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3941628a2766

Comment 10

[Farras Givari]

Hi team, is this eligible fot bounty?

Comment 11

[Farras Givari]

*for

Comment 12

[Rob Wu [:robwu]]

I'm not sure whether it meets the bar for a bug bounty payout, but that will be decided by the security team. You can find more information on the Bug Bounty program at https://www.mozilla.org/en-US/security/client-bug-bounty/

The report here encouraged me to take a closer look at the validation (and lack thereof) in Firefox and AMO, and we are fixing these. I expect that this bug will be assigned a CVE shortly before a build with this patch reaches release. That may be in 2 months (or 1 month).

Comment 13

[Farras Givari]

Okay, thanks for the update

Comment 14

[Simon Friedberger [:simonf]]

Rob, does the UI just become sluggish or is it actually impossible to remove such extensions again?

Comment 15

[Rob Wu [:robwu]]

Only the expanded details page of individual add-ons at about:addons become near-impossible to use due to the layout.

The sluggishness is only due to the excessive amount of data in the original test case (only after fixing bug 1939488), but I don't see a feasible way to load such an add-on in Firefox. Even on my very beefy M4 laptop, I'm hitting bug 1939488 that prevents me from loading the original test case.

But even without such an excessive long name, I do see other parts of the UI (not originally included in the report) that are concerning:

• The install-time dialog renders the name before the permissions. With a very long name, users cannot see the permissions. But as they cannot see the "Add" button, that might not be concerning.... unless they can convince the user to press Ctrl+A, which clicks the button that confirms add-on installation.
• The Extensions Button lists the name when the panel is opened. If the name is large, the extension entry becomes so large that the panel becomes unusable.

I'll attach test cases that you can use to experiment with the UI yourself if you'd like.

Comment 16

[Rob Wu [:robwu]]

Attachment: name_long.xpi

An extension package with a name of 40k characters.

To test:

1. Visit about:debugging and select the add-on.
2. Visit about:addons and play with the UI.

Test case v2, to test the install prompt:

1. Save the attached xpi file.
2. Visit about:config and set xpinstall.signatures.required to false (this is only available in Nightly, ESR and unbranded builds - NOT Release or Beta)
3. Visit the file://-URL where the xpi file was saved.
4. Click on the xpi link to the file from step 1.
5. The permission prompt appears. Press Ctrl+A to simulate clicking the Add button.
6. Visit about:addons and play with the UI

Note: At step 5, the install prompt does not show the Add button due to the excessive size of the name.

Generated as follows:

node -e 'console.log(JSON.stringify({name:"ABC ".repeat(10000), manifest_version:2, version: "1","browser_specific_settings":{"gecko":{"id":"name@long"}}},null,2));' > manifest.json

Comment 17

[Rob Wu [:robwu]]

Attachment: name_75.xpi

After the patch, the maximum name length is 75. This extension can be loaded following the same steps as comment 16.

manifest.json within the xpi file was generated with: node -e 'console.log(JSON.stringify({name:"ABC ".repeat(18) + "EOL", manifest_version:2, version: "1","browser_specific_settings":{"gecko":{"id":"name@75"}}},null,2));' > manifest.json

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta and ESR128 approval when you get a chance.

Comment 19

[Rob Wu [:robwu]]

This issue is supposedly mitigated by checks on the AMO side, but that is not the case (comment 5). While the client has a fix in place to mitigate any bypassed checks on the AMO side, IMO it would be ideal if the addons-linter and/or AMO were to be patched to correctly enforce the maximum lengths. That work is tracked by https://github.com/mozilla/addons/issues/15268.

This week my team (and the larger Add-ons group) are at a workweek, so to avoid additional pressure I'll hold off with uplift requests until next week.

Comment 20

[Daniel Veditz [:dveditz]]

Farras: thanks for reporting this bug. It does not qualify for a monetary bug bounty but we will be adding you to our hall of fame pages.

Comment 21

[Rob Wu [:robwu]]

Attachment: Bug 1939087 - Truncate long name and log warning

Original Revision: https://phabricator.services.mozilla.com/D233025

Comment 22

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Unbounded extension name length, which can impact the usability or clarity of the UI
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: https://bugzilla.mozilla.org/show_bug.cgi?id=1939087#c16
• Risk associated with taking this patch: Low
• Explanation of risk level: This patch enforces a maximum length by truncating at a limit that is higher than what is used in practice, per analysis at https://bugzilla.mozilla.org/show_bug.cgi?id=1939087#c5. The few cases that are affected are still usable, except their name is shortened.
• String changes made/needed: None
• Is Android affected?: yes

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/7464003b81b3