Closed
Bug 775435
Opened 12 years ago
Closed 12 years ago
GC hazard with cross-compartment sNPObjectMemberClass::npobjWrapper
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
Tracking | Status | |
---|---|---|
firefox14 | --- | unaffected |
firefox15 | --- | fixed |
firefox16 | + | fixed |
firefox17 | + | fixed |
firefox-esr10 | --- | unaffected |
People
(Reporter: bholley, Assigned: bholley)
References
Details
(Keywords: csectype-uaf, sec-high, Whiteboard: [advisory-tracking-])
Attachments
(1 file)
1.59 KB,
patch
|
benjamin
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
I was reading over my patches for bug 771202, and realized that I screwed this one up. :-( GetNPObjectWrapper currently returns an unwrapped JSObject*, since its primary consumer is GetNPObject, which just grabs the private (an NPObject*). But it's _also_ used in CreateNPObjectMember when storing |npobjWrapper|: https://hg.mozilla.org/mozilla-central/rev/0d561abceeb3#l1.234 I thought this was ok at the time, but sNPObjectMemberClass actually has a trace hook NPObjectMember_Trace, which means that npobjWrapper very much needs to be same-compartment with the NPObjectMember. Patch coming right up.
Assignee | ||
Comment 1•12 years ago
|
||
Attaching a patch. Flagging bsmedberg for review.
Attachment #643765 -
Flags: review?(benjamin)
Updated•12 years ago
|
status-firefox15:
--- → unaffected
status-firefox16:
--- → affected
status-firefox17:
--- → affected
tracking-firefox16:
--- → ?
tracking-firefox17:
--- → ?
Version: unspecified → 16 Branch
Comment 3•12 years ago
|
||
Comment on attachment 643765 [details] [diff] [review] Wrap-by-default in GetNPObjectWrapper. v1 Yeah. Damn this is complicated.
Attachment #643765 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Pushed to try here: https://tbpl.mozilla.org/?tree=Try&rev=83cf9dab23ac Looks green modulo infra orange, so pushing to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/6bff6810a82e
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6bff6810a82e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 643765 [details] [diff] [review] Wrap-by-default in GetNPObjectWrapper. v1 Waiting to hear whether this reduced crashes over in bug 774052, but we should take this on m-c and m-b regardless, because it's an obvious GC hazard. Requesting approval. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 771202 User impact if declined: Potential crashes and security bugs Testing completed (on m-c, etc.): baked on m-c over the weekend. The crash this may fix is being tracked in bug 774052. Risk to taking this patch (and alternatives if risky): Low risk. String or UUID changes made by this patch: None
Attachment #643765 -
Flags: approval-mozilla-beta?
Attachment #643765 -
Flags: approval-mozilla-aurora?
Comment 7•12 years ago
|
||
Comment on attachment 643765 [details] [diff] [review] Wrap-by-default in GetNPObjectWrapper. v1 [Triage Comment] Cleanup for uplifted patch, and low risk. Let's take these on branches.
Attachment #643765 -
Flags: approval-mozilla-beta?
Attachment #643765 -
Flags: approval-mozilla-beta+
Attachment #643765 -
Flags: approval-mozilla-aurora?
Attachment #643765 -
Flags: approval-mozilla-aurora+
Updated•12 years ago
|
Updated•12 years ago
|
Assignee | ||
Comment 8•12 years ago
|
||
Pushed to m-a: http://hg.mozilla.org/releases/mozilla-aurora/rev/da5ea41d129d
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 9•12 years ago
|
||
Pushed to m-b: http://hg.mozilla.org/releases/mozilla-beta/rev/7bf668f1e547
Updated•12 years ago
|
status-firefox-esr10:
--- → unaffected
Updated•12 years ago
|
status-firefox14:
--- → unaffected
Updated•12 years ago
|
Whiteboard: [advisory-tracking-]
Updated•12 years ago
|
Updated•12 years ago
|
Group: core-security
Comment 10•12 years ago
|
||
What is the visible impact while testing this on a build that reproduces this issue?
You need to log in
before you can comment on or make changes to this bug.
Description
•