The default bug view has changed. See this FAQ.
Bug 876762 (CVE-2013-1725)

ABORT: bad scope for new JSObjects: 'js::IsObjectInContextCompartment(lccx.GetScopeForNewJSObjects(), cx)' under ReparentWrapper / document.open

RESOLVED FIXED in Firefox 24, Firefox OS v1.1hd

Status

()

Core
XPConnect
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Ms2ger, Assigned: bholley)

Tracking

({csectype-uninitialized, sec-high})

Trunk
mozilla24
csectype-uninitialized, sec-high
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox21 wontfix, firefox22 wontfix, firefox23 wontfix, firefox24+ fixed, firefox-esr1724+ fixed, b2g1824+ fixed, b2g-v1.1hd fixed)

Details

(Whiteboard: [adv-main24+][adv-esr1709+])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 754887 [details]
Stack

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)
(Reporter)

Comment 2

4 years ago
Created attachment 754944 [details]
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.
Created attachment 755124 [details] [diff] [review]
Remove support for mScopeForNewJSObjects. v1
Attachment #755124 - Flags: review?(luke)
https://tbpl.mozilla.org/?tree=Try&rev=ebeafcde1234
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 7

4 years ago
Comment on attachment 755124 [details] [diff] [review]
Remove support for mScopeForNewJSObjects. v1

Good riddance!
Attachment #755124 - Flags: review?(luke) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd394e3894ac
Does this need to be backported?  I don't understand your first sentence in comment 6.
Keywords: csec-uninitialized
(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
Last Resolved: 4 years ago
status-firefox24: --- → fixed
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.
status-firefox21: --- → affected
status-firefox22: --- → affected
status-firefox23: --- → affected
status-firefox-esr17: --- → affected
Release management, we should take this on ESR. This got missed along the way.
tracking-firefox-esr17: --- → ?
Flags: needinfo?(release-mgmt)
Whiteboard: [adv-main24+]
status-b2g18: --- → affected
status-firefox21: affected → wontfix
status-firefox22: affected → wontfix
status-firefox23: affected → wontfix
tracking-b2g18: --- → ?
tracking-firefox24: --- → +
needinfo on :bholley to help with the backport on esr17 branch
tracking-firefox-esr17: ? → 24+
Flags: needinfo?(release-mgmt)

Updated

4 years ago
Flags: needinfo?(bobbyholley+bmo)
Created attachment 795527 [details] [diff] [review]
Remove support for mScopeForNewJSObjects on esr17. v1 r=luke

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)
https://hg.mozilla.org/releases/mozilla-esr17/rev/2c58bfd55714
status-firefox-esr17: affected → fixed
Whiteboard: [adv-main24+] → [adv-main24+][adv-esr1709+]
tracking-b2g18: ? → 24+
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?
https://hg.mozilla.org/releases/mozilla-b2g18/rev/0c8834ee5bb2
status-b2g18: affected → fixed
status-b2g-v1.1hd: --- → affected
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/0c8834ee5bb2
status-b2g-v1.1hd: affected → fixed
Group: core-security
You need to log in before you can comment on or make changes to this bug.