446538 - Hindi language pack on AMO breaks security updates

Status: RESOLVED DUPLICATE of bug 377881

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[Axel Hecht [:Pike]]

The language pack by RVKS Raman on is shipping a preference file overwriting general.useragent.locale.

unzip -p hindi__in__language_pack-1.0.0-fx.xpi defaults/preferences/hi-IN.js
pref("general.useragent.locale", "hi-IN");

Please update the language pack with one that doesn't, as that is breaking security updates for users of that language pack.

Comment 1

[Robert Strong (they/them - no direct email)]

Just did a quick audit of the first page returned for a search of "language packs" on AMO and the following ones also provide a general.useragent.locale pref

tamil__in__language_pack-1.0.0-fx.xpi
telugu__in__language_pack-1.0.0-fx.xpi
welsh_cy-gb_language_pack-1.0.0-fx.xpi

I suspect there are more

Comment 2

[Wil Clouser [:clouserw]]

CCing baz for a policy decision.  Feels like we should sandbox add-ons that break updates and contact the authors.

Comment 3

[Basil Hashem [:baz]]

OK, did a fulltext search of all lang packs in the system and here's what I found:

3790/defaults/preferences/prefs.js:pref("general.useragent.locale", "roa-ES-val");
4443/defaults/preferences/prefs.js:pref("general.useragent.locale", "roa-ES-val");
4444/defaults/preferences/cy-GB.js:pref("general.useragent.locale", "cy-GB");
5820/defaults/preferences/pref.js:pref("general.useragent.locale", "fr-FR"); [In sandbox already]
6045/defaults/preferences/hi-IN.js:pref("general.useragent.locale", "hi-IN");
6046/defaults/preferences/ta-IN.js:pref("general.useragent.locale", "ta-IN");
6047/defaults/preferences/te-IN.js:pref("general.useragent.locale", "te-IN");
7476/defaults/preferences/prefs.js:pref("general.useragent.locale", "roa-ES-val");
7557/defaults/preferences/prefs.js:pref("general.useragent.locale", "roa-ES-val");

Thankfully it doesn't seem that rampant. I've added this particular note to the policy @ http://wiki.mozilla.org/Update:Editors/ReviewingGuide#Reviewing_Language_Packs

I've temporarily sandboxed all these addons and notified the authors.

Comment 4

[Robert Strong (they/them - no direct email)]

Thanks Basil. I'm almost done with a patch for the client side (bug 446527) to prevent this from happening in the future.

Comment 5

[Bernat]

I think you're asking us to break our language packs. In my language pack I change all preferences that have the locale placeholder so updates and everything work as expected using a supported locale.

My language pack uses the "roa-ES-val" locale and if I remove that preference then users will have to change it manually from "about:config" as explained in sumoco (for advanced users only). Thus, they will break updates unknowingly.

The "Quick Locale Switcher" extension doesn't work as it doesn't support our locale, besides it does the same you're asking us not to do.

I think this bug is a workaround to another 2 Firefox/Mozilla bugs, bug 377881 and another issue I can't find right now in Bugzilla that is having locale placeholders in preference URLs instead of using server-side preferred locale detection.

Comment 6

[Axel Hecht [:Pike]]

Bernat, that sounds like you're overriding the preference for security updates, which is even more scary.

I know that the langpack install experience isn't great, but there needs to be a balance between the risk imposed and the benefit. We did change the format of the update url at least once, on a stable release chain, and we may do again.

If the quick locale switcher doesn't work, I bet the locale switcher by Benjamin does, https://addons.mozilla.org/en-US/firefox/addon/356.

Comment 7

[Bernat]

I don't see how trying to fix something is worst than letting it broken, but maybe you should be checking those preferences too since someone might use them for bad, or you should rather check for a set of allowed preferences. Maybe, having those URLs so easily replaceable should be seen as a security flow since it's easy for any non-secured extension to change them.

Anyway, you're right, that switcher works well. I guess changing the pref from javascript is what makes it work, but since langpacks can't include javascript we're done.

JFYI, I think requiring a 2nd extension just to use a new language is overkill, I suggest to integrate Benjamin's extension in Firefox or fix bug 377881 taking care of locale placeholders in urls too.

Comment 8

[Bernat]

Sorry, I meant *security flaw*.

Comment 9

[Robert Strong (they/them - no direct email)]

Bernat, bug 446527 will make it so we don't rely on that pref but even after that lands the existing users of the language pack will need to either uninstall it or upgrade to a version without that pref. After it lands you could also provide a version of the language pack that has the pref and has a compatibility minVersion that is only compatible with the Firefox version that has the fix.

Comment 10

[Bernat]

Robert, since it seems now mandatory not to include that pref, I've already uploaded a new version without it.

Axel and Basil, I'd like you understand we don't like the way things are working right now,  I mean we just used that workaround to make it easier for users without much knowledge about extensions. Having to deal with so much pref variables to make updates work is really a pain.

Maybe, including a message with a link to the locale switcher in the language pack template page in a.m.o. would be a little enhancement for users in the meantime.

Comment 11

[Jeferson Hultmann]

Locale Switcher is a UI for editing general.useragent.locale. Shouldn't it be sandboxed as well?

Comment 12

[Bernat]

For other language packagers, the locale switcher extension won't work in iceweasel/icedove if you don't provide this preference in your language pack:
pref("intl.locale.matchOS", false);

Hope it helps.

Comment 13

[Michael Morgan [:morgamic]]

Resolving as a dupe of bug 377881.  Please re-open if there is more to do for this bug.