Closed Bug 1315435 (CVE-2016-9903) Opened 8 years ago Closed 8 years ago

Intentional XSS injection hole in add-ons SDK

Categories

(Add-on SDK Graveyard :: General, defect)

Product:
Add-on SDK Graveyard
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox50 fixed, firefox51 fixed, firefox52 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

Details

(Keywords: sec-moderate, Whiteboard: [qa-][adv-main50.1+])

Attachments

(1 file)

Fix intentional, gaping XSS hole.
4.50 KB, patch
mossop
: review+
gchang
: approval-mozilla-beta+
ritu
: approval-mozilla-release+
abillings
: sec-approval+
Details | Diff | Splinter Review
I honestly don't know if or how this exploitable, but the SDK bundles a world-accessible file with an intentional HTML injection vulnerability: http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/addon-sdk/source/lib/sdk/ui/frame/view.html#17 document.documentElement.innerHTML = atob(location.hash.substr(1)); This file is accessible to the entire web at resource://gre/modules/commonjs/sdk/ui/frame/view.html in the stock browser, and at various other locations in add-ons with bundled copies of the SDK. In the latter case, it can probably at the least be used to access the add-on's localStorage, or to trick unwary content scripts. It's probably exploitable in other ways too, though.
Attachment #8807910 - Flags: review?(dtownsend)
Assignee: nobody → kmaglione+bmo
Status: NEW → ASSIGNED
Jorge, Can you please look into notifying the authors of AMO add-ons that bundle this file that they need to remove their bundled copies of the SDK, and block the ones that don't do so promptly? This could be an especially dangerous exploit in the case of add-ons that use localStorage or load content scripts into URLs belonging to their add-on.
Flags: needinfo?(jorge)
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. Review of attachment 8807910 [details] [diff] [review]: ----------------------------------------------------------------- ::: addon-sdk/source/lib/sdk/ui/frame/view.html @@ +2,5 @@ > +<html> > + <head> > + <script> > + // HACK: This is not an ideal way to deliver chrome messages > + // to a innef frame content but seems only way that would I know you didn't write it but let's fix the innef typo here
Attachment #8807910 - Flags: review?(dtownsend) → review+
Somewhat arbitrarily calling this sec-high, since I'm not actually sure of the implications.
Keywords: sec-high
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. [Security approval request comment] How easily could an exploit be constructed based on the patch? Not easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? All. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The patch should apply cleanly on all branches. How likely is this patch to cause regressions; how much testing does it need? Very unlikely.
Attachment #8807910 - Flags: sec-approval?
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. sec-approval+ Do we need to take this patch on branches?
Attachment #8807910 - Flags: sec-approval? → sec-approval+
(In reply to Al Billings [:abillings] from comment #6) > Do we need to take this patch on branches? Probably. I don't have a good proof of concept exploit for the in-tree version of this, but anyone who put in some effort probably could. The fix is fairly low-risk, in any case.
I guess Jorge is busy traveling. Andreas or Andrew, can one of you please take care of the add-ons this affects, per comment 2? Thanks
Flags: needinfo?(awilliamson)
Flags: needinfo?(awagner)
Flags: needinfo?(awilliamson)
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. Approval Request Comment [Feature/regressing bug #]: Bug 787390 [User impact if declined]: This is an easily exploitable XSS/privilege escalation vulnerability, but the potential impact is unclear. [Describe test coverage new/current, TreeHerder]: There are tests for this feature, but not for this security issue. [Risks and why]: Low. This is a fairly simple change that removes a DOM to HTML to DOM round trip, and an accompanying security hole, but otherwise changes very little. [String/UUID change made/needed]: None.
Attachment #8807910 - Flags: approval-mozilla-release?
Attachment #8807910 - Flags: approval-mozilla-beta?
Attachment #8807910 - Flags: approval-mozilla-aurora?
20 add-ons are affected, we'll reach out to them next week.
https://hg.mozilla.org/mozilla-central/rev/ef58f71d8e7b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Developer outreach blocked on bug 1317288.
Flags: needinfo?(jorge)
Attachment #8807910 - Flags: approval-mozilla-aurora?
web content can't link or open resource:// or chrome:// urls. How would this be exploited by web content? Do we have features that take arbitrary web content (from messages? urls?) and passes them to a frame containing this content? IOW is this automatically a vulnerability, or is it a foot-gun that requires the add-on to also do something specific and probably ill-advised?
Hm. You're right, web content can only directly load the URLs as resources, not as documents, so I think it requires another vulnerability to exploit.
So, do we still need to reach out to developers and block the affected add-ons if they don't respond/fix it?
Flags: needinfo?(awagner) → needinfo?(kmaglione+bmo)
Group: toolkit-core-security → core-security-release
Hi Kris, When you nominate the uplift request, aurora is 51, beta is for 50, and release is for 49. Since the merge days is passed, is this worth uplifting to 51 beta?
(In reply to Andreas Wagner [:TheOne] from comment #17) > So, do we still need to reach out to developers and block the affected > add-ons if they don't respond/fix it? Yes. The issue isn't as severe as I originally thought, but it's still severe. (In reply to Gerry Chang [:gchang] from comment #18) > Hi Kris, > When you nominate the uplift request, aurora is 51, beta is for 50, and > release is for 49. Since the merge days is passed, is this worth uplifting > to 51 beta? Yes. I canceled the aurora uplift request after the merge, but I'd still like to uplift to the other channels.
Flags: needinfo?(kmaglione+bmo)
Developers have been notified in bug 1318453.
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. Fix a sec-high. Beta51+. Should be in 51 beta 2. NI Ritu for considering uplifting to 50.1/50 dot release.
Flags: needinfo?(rkothari)
Attachment #8807910 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Is there necessary any manual testing around this bug? If yes, could you please provide some reliable steps? Or it would be preferable to perform exploratory testing around the add-ons mentioned in Comment 14?
Flags: needinfo?(kmaglione+bmo)
There's probably not much point in manual testing. It might be a good idea to do some testing of add-ons that (directly or indirectly) use the ui/frame module, but if this were going to cause breakage there, we almost certainly have seen it by now.
Flags: needinfo?(kmaglione+bmo)
Whiteboard: [qa-]
I would prefer to keep the fixes for 50.0.1 release (planned to gtb and push this week) to a minimal set of required fixes. I think we should take this in the planned 50.1.0 release. Please let me know if there are any concerns.
Flags: needinfo?(rkothari)
50.1.0 sounds fine to me.
Based on comment 15, dropping this to sec-moderate, since it should only be exploitable in combination with other vulnerabilities.
Keywords: sec-high → sec-moderate
Comment on attachment 8807910 [details] [diff] [review] Fix intentional, gaping XSS hole. Meets the triage bar for 50.1.0
Attachment #8807910 - Flags: approval-mozilla-release? → approval-mozilla-release+
Whiteboard: [qa-] → [qa-][adv-main50.1+]
Alias: CVE-2016-9903
Affected add-on versions blocked in bug 1325060.
See Also: → 1325060
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: