Closed Bug 852134 Opened 7 years ago Closed 7 years ago
Request DROPs without clearing its wrappercache
I found this while auditing other classes that use NS_IMPL_CYCLE_COLLECTION_TRACE_JSVAL_MEMBER_CALLBACK. The calls to UnrootResult outside of Unlink and the destructor leave dangling pointers to the wrapper cache. Same issue as bug 851996, basically the same fix should work. I'll double check other classes today, though we've already fixed related sec-crits for them, so hopefully they are okay. Being able to double HOLD and DROP really makes this easier to fix. Unfortunately this appears to affect everything. Is SmsRequest something we should fix in ESR17, or is it not web exposed in desktop?
Component: DOM: Events → DOM: Device Interfaces
It should be b2g only.
I see at least one SMS related bug on Android (bug 719795).
Assignee: nobody → continuation
Mounir, do we ship SMS in some kind of active capacity on Android or Desktop? I'm wondering whether we want to patch this on beta. If we do, then I may have to wait until next cycle to land this.
WebSMS API on Android is being a pref and the real implementation is behind a compilation flag which is not enabled on any official Mozilla build. That means that turning on the pref will just give you an empty API shell. Though, that bug would apply with the pref turned on but I guess we shouldn't land something on ESR for a pref'd off feature?
Great, thanks! Yeah, no need to patch it where it is prefed off, so it sounds like b2g and trunk are the only relevant branches here.
7 years ago
Comment on attachment 726205 [details] [diff] [review] DROP less aggressively Review of attachment 726205 [details] [diff] [review]: ----------------------------------------------------------------- Try run looked okay, modulo some X failures in another patch. I'll retry it at some point. https://tbpl.mozilla.org/?tree=Try&rev=cda337ecb483
Attachment #726205 - Flags: review?(bugs)
Comment on attachment 726205 [details] [diff] [review] DROP less aggressively [Security approval request comment] How easily could an exploit be constructed based on the patch? probably not too 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? SMS is disabled everywhere aside from b2g, so the only relevant branches for this are b2g18 and trunk. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should be trivial to backport to b2g18. How likely is this patch to cause regressions; how much testing does it need? It should be pretty safe.
Attachment #726205 - Flags: sec-approval?
Attachment #726205 - Flags: sec-approval? → sec-approval+
Comment on attachment 726205 [details] [diff] [review] DROP less aggressively https://hg.mozilla.org/integration/mozilla-inbound/rev/561ac0327dc9 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): landing of SMS User impact if declined: possibly security bugs Testing completed: try run Risk to taking this patch (and alternatives if risky): should be pretty low risk String or UUID changes made by this patch: none
Attachment #726205 - Flags: approval-mozilla-b2g18?
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Attachment #726205 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/2de7d72b757a What about v1.0.1?
1.0.1 is certainly affected, I don't know how landing goes for that.
We still have the opportunity to get this all the way up to v1.0.1. Marking as tef+ and CCing Ryan.
blocking-b2g: tef? → tef+
You need to log in before you can comment on or make changes to this bug.