Don't use localized prefs for "Get (extensions|themes)" URLs

RESOLVED FIXED in 0.8

Status

Calendar
Preferences
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: philor, Assigned: philor)

Tracking

({fixed1.8.1.12})

unspecified
fixed1.8.1.12
Dependency tree / graph

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
Created attachment 289862 [details] [diff] [review]
Fix v.1

Sunbird is the only (shipping, Composer "uses" them too) consumer for the localized versions of extensions.getMoreExtensionsURL and extensions.getMoreThemesURL in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties. Firefox and Thunderbird both switched to using a formatted https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/extensions/ URL in their preferences before their 2.0 versions, so the only place anyone ever sees the value of those localized properties is in Sunbird. 

That doesn't do a thing for the quality of the (pointless) localization - they are a random assortment of various old versions of the AMO URLs, but what's most significant is that they are randomly http or https, making Sunbird users the only ones at risk of bug 384897 man-in-the-middle attacks. Since there's no need to have them localized (no locale is going to use a different URL, and if they do you don't want them to), and even if you did want them localized you wouldn't want that localization shared with every toolkit consumer, so you would need an app-specific .properties file, the best (as in, likely to actually land) way to fix bug 384897 for Sunbird is just to switch to a non-localized URL in prefs.
Attachment #289862 - Flags: review?(daniel.boelzle)
(Assignee)

Updated

11 years ago
Blocks: 384897
(Assignee)

Updated

11 years ago
Blocks: 405068

Comment 1

11 years ago
Sounds reasonable to me. Simon, what's your opinion on this?

Comment 2

11 years ago
I'm fine with this patch as well. Phil, I assume that you've tested this with current Sunbird nightlies?
(Assignee)

Comment 3

11 years ago
Yep, I tested, though you can test yourself without even needing to build - change the pref extensions.getMoreExtensionsURL to something like http://www.mozilla.org/ so you can see it's being used instead of the localized version, then with something like the LiveHTTPHeaders extension installed in your browser so you can see where you go before the AMO redirect, change the pref to https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/extensions/ and you'll see that a branch Sunbird is going to https://en-us.add-ons.mozilla.com/en-US/sunbird/0.8pre/extensions/ before being redirected to https://addons.mozilla.org/en-US/sunbird/browse/type:1

I'm not absolutely sure this is the best solution - I want too much, so I want there to only be one copy of that AMO URI in the tree, but to still make it obvious to anyone doing a toolkit app that they can use an http URI in their app's prefs, or a non-localized  URI in a properties file somewhere in their content, or a localized URI in their localized app files, so I'm still waffling a bit in bug 405068, but I've got my orders to fix that one way or another by 1.9M11, so I'd appreciate an r+ here even though the right answer might be changing your pref URL to chrome://mozapps/extensions/urls.properties (with, of course, an #ifdef to change to the https: URI for 1.8, where you can't have that new file).

Comment 4

11 years ago
Phil, you confuse me a bit. So the attached patch is for trunk only?
(Assignee)

Comment 5

11 years ago
Spreading confusion may not be my goal, but it's certainly my chief talent.

The attached patch will work perfectly on the trunk and in 1.8, and has absolutely no dependencies. It will make no visible changes. The only thing that will change is that those locales which didn't change their localized URLs from http to https will not be vulnerable to having a click on "Get Extensions" hijacked by someone pretending to be http://de.add-ons.mozilla.com/ or http://fr.add-ons.mozilla.com/, because the initial connection will be to the https URI from the prefs.

Comment 6

11 years ago
Comment on attachment 289862 [details] [diff] [review]
Fix v.1

r=dbo
Attachment #289862 - Flags: review?(daniel.boelzle) → review+
(Assignee)

Comment 7

11 years ago
calendar/sunbird/app/profile/sunbird.js 1.47
calendar/sunbird/app/profile/sunbird.js 1.17.2.23
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.12
Resolution: --- → FIXED
Target Milestone: --- → 0.8
You need to log in before you can comment on or make changes to this bug.