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

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: bholley)

Tracking

({csectype-uninitialized, sec-high})

Trunk
mozilla24
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)

Posted 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
Assignee

Updated

6 years ago
Group: core-security
Did you mean to attach something else, Ms2ger, that looks like a patch...
Flags: needinfo?(Ms2ger)
Reporter

Comment 2

6 years ago
Posted file Stack
Yep.
Attachment #754887 - Attachment is obsolete: true
Flags: needinfo?(Ms2ger)
Assignee

Comment 3

6 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

Updated

6 years ago
Attachment #755124 - Attachment description: Remove support for mScopeForJSObjects. v1 → Remove support for mScopeForNewJSObjects. v1
Assignee

Comment 6

6 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 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.
Assignee

Comment 10

6 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.
https://hg.mozilla.org/mozilla-central/rev/dd394e3894ac
Assignee: nobody → bobbyholley+bmo
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee

Comment 12

6 years ago
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?
Assignee

Comment 14

6 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.
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)
Assignee

Comment 17

6 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

6 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?
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.
Assignee

Comment 22

6 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.
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)
Assignee

Comment 25

6 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

6 years ago
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.