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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 + fixed
firefox-esr17 24+ fixed
b2g18 24+ fixed
b2g-v1.1hd --- fixed

People

(Reporter: Ms2ger, Assigned: bholley)

Details

(Keywords: csectype-uninitialized, sec-high, Whiteboard: [adv-main24+][adv-esr1709+])

Attachments

(3 files, 1 obsolete file)

Attached file Stack (obsolete) —
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
Group: core-security
Did you mean to attach something else, Ms2ger, that looks like a patch...
Flags: needinfo?(Ms2ger)
Attached file Stack
Yep.
Attachment #754887 - Attachment is obsolete: true
Flags: needinfo?(Ms2ger)
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.
Attachment #755124 - Attachment description: Remove support for mScopeForJSObjects. v1 → Remove support for mScopeForNewJSObjects. v1
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 on attachment 755124 [details] [diff] [review]
Remove support for mScopeForNewJSObjects. v1

Good riddance!
Attachment #755124 - Flags: review?(luke) → review+
Does this need to be backported?  I don't understand your first sentence in comment 6.
(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.
https://hg.mozilla.org/mozilla-central/rev/dd394e3894ac
Assignee: nobody → bobbyholley+bmo
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
The failing mechanism is being removed, so I don't think we need test coverage.
Flags: in-testsuite? → in-testsuite-
(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?
(In reply to Al Billings [:abillings] from comment #13)
> How far back does this need to go? Does it affect ESR17?

It does.
Release management, we should take this on ESR. This got missed along the way.
Flags: needinfo?(release-mgmt)
Whiteboard: [adv-main24+]
needinfo on :bholley to help with the backport on esr17 branch
Flags: needinfo?(release-mgmt)
Flags: needinfo?(bobbyholley+bmo)
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+
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?
Alias: CVE-2013-1725
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+
Flags: needinfo?(bobbyholley+bmo)
Whiteboard: [adv-main24+] → [adv-main24+][adv-esr1709+]
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.
(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.
Thanks Bobby. QA is going to pass on verification based on that info.
NI on :bholley for a backport on b2g18.
Flags: needinfo?(bobbyholley+bmo)
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?
Flags: needinfo?(bobbyholley+bmo)
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?
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: