Closed Bug 1235876 (CVE-2016-1948) Opened 5 years ago Closed 5 years ago
LWT permissions exposed to man-in-the-middle attacks
To install a lightweight theme, we listen for custom events from addons.mozilla.org, and we check the document URI to make sure these events are actually coming from addons.mozilla.org: https://dxr.mozilla.org/mozilla-central/rev/9ddf0da90fb3bc1ae29966dc596013fc54a44bd2/mobile/android/chrome/content/browser.js#3251-3259 However, we don't check the scheme to ensure it's https. We should do that. Desktop already does this: https://dxr.mozilla.org/mozilla-central/rev/9ddf0da90fb3bc1ae29966dc596013fc54a44bd2/browser/base/content/browser-addons.js#631-647 To fix this bug, make the mobile code do what the desktop code does.
Actually, this is a security bug for all add-ons. See the desktop bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1128126
Assignee: nobody → margaret.leibovic
Summary: Check scheme before handling lightweight theme events → Addon permissions exposed to man-in-the-middle attacks
Whiteboard: [lang=js][good first bug]
Actually, the patch in bug 1128126 contains a fix for XPIProvider.jsm, which protects Fennec for the non-LWT case. But I'll still write a patch for the LWT case.
See Also: → CVE-2015-0812
Summary: Addon permissions exposed to man-in-the-middle attacks → LWT permissions exposed to man-in-the-middle attacks
Attachment #8702984 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/fe8054f2ac3e765c8d3b51df78222cd0777c4163 Bug 1235876 - Check scheme before handling lightweight theme events. r=Gijs
Comment on attachment 8702984 [details] [diff] [review] Check scheme before handling lightweight theme events. r=Gijs Looking at bug 1128126 comment 16, this is likely a sec-low, given that it only affects lightweight themes (not add-ons). So I went ahead and landed it. Approval Request Comment [Feature/regressing bug #]: None. [User impact if declined]: Potential for MTIM attacks when installing a lightweight theme. [Describe test coverage new/current, TreeHerder]: No automated tests, verified locally this doesn't regress LWT installs. [Risks and why]: Low-risk, adds check to make sure URI installing LWTs is https. [String/UUID change made/needed]: None.
Attachment #8702984 - Attachment description: CCheck scheme before handling lightweight theme events. r=Gijs → Check scheme before handling lightweight theme events. r=Gijs
Comment on attachment 8702984 [details] [diff] [review] Check scheme before handling lightweight theme events. r=Gijs Sec issue with a simple fix, Beta44+, Aurora45+
Whiteboard: [adv-main44+] → [post-critsmash-triage][adv-main44+]
You need to log in before you can comment on or make changes to this bug.