Closed
Bug 876762
(CVE-2013-1725)
Opened 11 years ago
Closed 11 years ago
ABORT: bad scope for new JSObjects: 'js::IsObjectInContextCompartment(lccx.GetScopeForNewJSObjects(), cx)' under ReparentWrapper / document.open
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
People
(Reporter: Ms2ger, Assigned: bholley)
Details
(Keywords: csectype-uninitialized, sec-high, Whiteboard: [adv-main24+][adv-esr1709+])
Attachments
(3 files, 1 obsolete file)
3.13 KB,
text/plain
|
Details | |
17.09 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
15.58 KB,
patch
|
bholley
:
review+
lsblakk
:
approval-mozilla-esr17+
|
Details | Diff | Splinter Review |
STR: 1. Paste the following into the live dom viewer <iframe src=about:blank></iframe> ...<script> onload = function() { document.open() } </script> 2. Type something random on the empty line
Assignee | ||
Updated•11 years ago
|
Group: core-security
Comment 1•11 years ago
|
||
Did you mean to attach something else, Ms2ger, that looks like a patch...
Flags: needinfo?(Ms2ger)
Reporter | ||
Comment 2•11 years ago
|
||
Yep.
Attachment #754887 -
Attachment is obsolete: true
Flags: needinfo?(Ms2ger)
Assignee | ||
Comment 3•11 years ago
|
||
So, this is basically the result of the stuff in BindingUtils.cpp creating an XPCLazyCallContext without either passing in a wrapper or calling SetScopeForNewJSObjects, which is verboten. But I'm pretty sure this whole mechanism is unnecessary since CPG, since we can now just grab the scope off of cx->compartment. I'll attach a patch.
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #755124 -
Flags: review?(luke)
Assignee | ||
Comment 5•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=ebeafcde1234
Assignee | ||
Updated•11 years ago
|
Attachment #755124 -
Attachment description: Remove support for mScopeForJSObjects. v1 → Remove support for mScopeForNewJSObjects. v1
Assignee | ||
Comment 6•11 years ago
|
||
So, until the recent rooting work, it looks like mScopeForNewJSObjects would remain uninitialized for XPCCallContexts without a wrapper unless somebody explicitly set it, and it's clear that here (and perhaps elsewhere) we don't. I'm not really sure what the severity of "entering the compartment of a JSObject* that is uninitialized stack memory" is, but I'm going to guess sec-high to be safe. This would likely be pretty hard to exploit though.
Keywords: sec-high
Comment 7•11 years ago
|
||
Comment on attachment 755124 [details] [diff] [review] Remove support for mScopeForNewJSObjects. v1 Good riddance!
Attachment #755124 -
Flags: review?(luke) → review+
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd394e3894ac
Comment 9•11 years ago
|
||
Does this need to be backported? I don't understand your first sentence in comment 6.
Keywords: csec-uninitialized
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #9) > Does this need to be backported? I don't understand your first sentence in > comment 6. When these objects became rooted, they became null-initialized. We could try to spot-fix the sketchy parts on branches, but I think it's safe to just backport this patch.
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dd394e3894ac
Assignee: nobody → bobbyholley+bmo
Status: NEW → RESOLVED
Closed: 11 years ago
status-firefox24:
--- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 12•11 years ago
|
||
The failing mechanism is being removed, so I don't think we need test coverage.
Flags: in-testsuite? → in-testsuite-
Comment 13•11 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #10) > (In reply to Andrew McCreight [:mccr8] from comment #9) > > Does this need to be backported? I don't understand your first sentence in > > comment 6. > > When these objects became rooted, they became null-initialized. > > We could try to spot-fix the sketchy parts on branches, but I think it's > safe to just backport this patch. How far back does this need to go? Does it affect ESR17?
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Al Billings [:abillings] from comment #13) > How far back does this need to go? Does it affect ESR17? It does.
status-firefox21:
--- → affected
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox-esr17:
--- → affected
Comment 15•11 years ago
|
||
Release management, we should take this on ESR. This got missed along the way.
tracking-firefox-esr17:
--- → ?
Updated•11 years ago
|
Flags: needinfo?(release-mgmt)
Updated•11 years ago
|
Whiteboard: [adv-main24+]
Updated•11 years ago
|
Comment 16•11 years ago
|
||
needinfo on :bholley to help with the backport on esr17 branch
Flags: needinfo?(release-mgmt)
Updated•11 years ago
|
Flags: needinfo?(bobbyholley+bmo)
Assignee | ||
Comment 17•11 years ago
|
||
I can't compile esr17 locally, so I don't know if this compiles. But we don't have try either, so I guess we should just crashland it.
Attachment #795527 -
Flags: review+
Assignee | ||
Comment 18•11 years ago
|
||
Comment on attachment 795527 [details] [diff] [review] Remove support for mScopeForNewJSObjects on esr17. v1 r=luke [Approval Request Comment] User impact if declined: sec Fix Landed on Version: 24 Risk to taking this patch (and alternatives if risky): Lowish risk String or UUID changes made by this patch: None
Attachment #795527 -
Flags: approval-mozilla-esr17?
Updated•11 years ago
|
Alias: CVE-2013-1725
Comment 19•11 years ago
|
||
Comment on attachment 795527 [details] [diff] [review] Remove support for mScopeForNewJSObjects on esr17. v1 r=luke There's still time so please do land and check that the landing sticks on esr17.
Attachment #795527 -
Flags: approval-mozilla-esr17? → approval-mozilla-esr17+
Updated•11 years ago
|
Flags: needinfo?(bobbyholley+bmo)
Updated•11 years ago
|
Whiteboard: [adv-main24+] → [adv-main24+][adv-esr1709+]
Updated•11 years ago
|
Comment 21•11 years ago
|
||
Comment 12 says that by removing some code, this no longer needs a test. If that is the case, let us know. If not, QA would like to verify that it's fixed. Thanks.
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Matt Wobensmith from comment #21) > Comment 12 says that by removing some code, this no longer needs a test. If > that is the case, let us know. If not, QA would like to verify that it's > fixed. Thanks. yes, this patch doesn't need verification.
Comment 23•11 years ago
|
||
Thanks Bobby. QA is going to pass on verification based on that info.
Assignee | ||
Comment 25•11 years ago
|
||
Comment on attachment 795527 [details] [diff] [review] Remove support for mScopeForNewJSObjects on esr17. v1 r=luke I would guess that the esr17 fix will also apply to b2g18. Let me know if there are nontrivial conflicts.
Attachment #795527 -
Flags: approval-mozilla-b2g18?
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(bobbyholley+bmo)
Comment 26•11 years ago
|
||
Comment on attachment 795527 [details] [diff] [review] Remove support for mScopeForNewJSObjects on esr17. v1 r=luke Got a+ from bajaj over email to land this.
Attachment #795527 -
Flags: approval-mozilla-b2g18?
Comment 27•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/0c8834ee5bb2
status-b2g-v1.1hd:
--- → affected
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•