Closed Bug 1235876 (CVE-2016-1948) Opened 5 years ago Closed 5 years ago

LWT permissions exposed to man-in-the-middle attacks

Categories

(Firefox for Android :: General, defect)

35 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main44+])

Attachments

(1 file)

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
Group: firefox-core-security
Mentor: 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)
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 - Flags: approval-mozilla-beta?
Attachment #8702984 - Flags: approval-mozilla-aurora?
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+
Attachment #8702984 - Flags: approval-mozilla-beta?
Attachment #8702984 - Flags: approval-mozilla-beta+
Attachment #8702984 - Flags: approval-mozilla-aurora?
Attachment #8702984 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/fe8054f2ac3e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Group: firefox-core-security → core-security-release
Keywords: sec-low
Whiteboard: [adv-main44+]
Alias: CVE-2016-1948
Flags: qe-verify-
Whiteboard: [adv-main44+] → [post-critsmash-triage][adv-main44+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.