Assertion failure: wrapper->isWrapper() setting domain

VERIFIED FIXED in Firefox 16

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: bc, Assigned: bholley)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla18
assertion, csectype-sop, regression, sec-high
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox15 unaffected, firefox16+ verified, firefox17+ verified, firefox-esr10 unaffected)

Details

(Whiteboard: [js:p1:fx18][advisory-tracking-], URL)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
1. http://maharashtratimes.indiatimes.com/articlelistmt/2499221.cms

2. Assertion failure: wrapper->isWrapper(), at /work/mozilla/builds/aurora/mozilla/js/src/jswrapper.cpp:69

Nightly Windows, Linux, Mac
 
Operating system: Linux
                  0.0.0 Linux 2.6.32-279.5.2.el6.i686 #1 SMP Tue Aug 14 12:48:24 EDT 2012 i686
CPU: x86
     GenuineIntel family 6 model 37 stepping 1
     2 CPUs

Crash reason:  SIGSEGV
Crash address: 0x0

Thread 0 (crashed)
 0  libxul.so!js::Wrapper::wrappedObject [jswrapper.cpp : 69 + 0x25]
    eip = 0x036ff08b   esp = 0xbffa2520   ebp = 0xbffa2538   ebx = 0x041d6e00
    esi = 0x0adb2fe8   edi = 0x00000001   eax = 0x00000000   ecx = 0x00c9434c
    edx = 0x00000000   efl = 0x00210286
    Found by: given as instruction pointer in context
 1  libxul.so!js::RemapWrapper [jswrapper.cpp : 1077 + 0xa]
    eip = 0x03702e2a   esp = 0xbffa2540   ebp = 0xbffa25c8   ebx = 0x041d6e00
    esi = 0x0adb2fe8   edi = 0x00000001
    Found by: call frame info
 2  libxul.so!js::RecomputeWrappers [jswrapper.cpp : 1148 + 0x18]
    eip = 0x03703299   esp = 0xbffa25d0   ebp = 0xbffa26a8   ebx = 0x041d6e00
    esi = 0x0adb2fe8   edi = 0x00000001
    Found by: call frame info
 3  libxul.so!nsPrincipal::SetDomain [nsPrincipal.cpp : 995 + 0x3c]
    eip = 0x01fa532b   esp = 0xbffa26b0   ebp = 0xbffa2708   ebx = 0x041d6e00
    esi = 0x0adb2fe8   edi = 0x01fa5208
    Found by: call frame info
 4  libxul.so!nsHTMLDocument::SetDomain [nsHTMLDocument.cpp : 1012 + 0x27]
    eip = 0x01bc8df5   esp = 0xbffa2710   ebp = 0xbffa2928   ebx = 0x041d6e00
    esi = 0x0adb2fe8   edi = 0x01fa5208
    Found by: call frame info
 5  libxul.so!nsIDOMHTMLDocument_SetDomain [dom_quickstubs.cpp : 13793 + 0x22]
    eip = 0x0235b491   esp = 0xbffa2930   ebp = 0xbffa29a8   ebx = 0x041d6e00
    esi = 0x01bc88ce   edi = 0xb2039e20
    Found by: call frame info

Found regression between 20120816175051-20120817191801
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a79132ac2f05&tochange=812ea773f166
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/08/2012-08-17-mozilla-central-debug/firefox-17.0a1.en-US.debug-linux-i686.tar.bz2
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/08/2012-08-18-mozilla-central-debug/firefox-17.0a1.en-US.debug-linux-i686.tar.bz2
(Reporter)

Comment 1

6 years ago
Doh. actually this is in Beta/16, and Aurora/17 as well. Looks like Bug 781476  to me.
Blocks: 781476
(Assignee)

Comment 2

6 years ago
This is an awesome bug report. Thanks Bob.
(Assignee)

Comment 3

6 years ago
Let's make this s-s for now.
Group: core-security
(Assignee)

Comment 4

6 years ago
The issue here is that our WN-sharing hack in bug 781476 depends on a subsumes check between the two compartments, and subsumes is affected by document.domain. So suppose we've got a cross-compartment wrapper to a same-origin, cross-compartment, spineless-PreCreate object before setting document.domain. When we set document.domain, we recompute all the cross-compartment wrappers, which causes us to call wrap() a second time on the underlying object. But since the compartments are no longer same-origin per-subsumes, we'll just build a new WN in WrapperFactory::PrepareForWrapping, which breaks the assumptions in js::RemapWrapper. This explains the crashes in bug 787637.

The solution here to ignore domain in the expando-sharing computation. We still won't leak expandos to the wrong origin (since our security wrapper computation still considers document.domain - I've added an explicit test for this), and I don't think it's a big deal if we fail to share expandos after document.domain shenanigans.
(Assignee)

Updated

6 years ago
Blocks: 787637
No longer blocks: 781476
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
(Assignee)

Comment 5

6 years ago
Created attachment 659845 [details] [diff] [review]
Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. v1

Attaching a patch. Flagging mrbkap for review.
Attachment #659845 - Flags: review?(mrbkap)
Whiteboard: [js:p1:fx18]
Tracking for FF16,FF17 as this blocks Bug 787637 which is being tracked as well.
tracking-firefox16: ? → +
tracking-firefox17: ? → +

Updated

6 years ago
Attachment #659845 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

6 years ago
Assignee: general → bobbyholley+bmo
https://hg.mozilla.org/mozilla-central/rev/132b9b646ebc
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Please nominate for aurora/beta approval once comfortable with the changes on Nightly.
(Assignee)

Comment 11

6 years ago
Comment on attachment 659845 [details] [diff] [review]
Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. v1

[Approval Request Comment] - bug 789713
Bug caused by (feature/regressing bug #): combination of bug 655649 and bug 781476
User impact if declined: crashes (bug 787637), potential security issues
Testing completed (on m-c, etc.): baked a few days on m-c
Risk to taking this patch (and alternatives if risky): Low risk. No obvious alternatives.
String or UUID changes made by this patch: None
Attachment #659845 - Flags: approval-mozilla-beta?
Attachment #659845 - Flags: approval-mozilla-aurora?

Updated

6 years ago
status-firefox-esr10: --- → unaffected
Comment on attachment 659845 [details] [diff] [review]
Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. v1

[Triage Comment]
Approving this for branches given the security and stability implications.
Attachment #659845 - Flags: approval-mozilla-beta?
Attachment #659845 - Flags: approval-mozilla-beta+
Attachment #659845 - Flags: approval-mozilla-aurora?
Attachment #659845 - Flags: approval-mozilla-aurora+
Keywords: verifyme

Comment 14

6 years ago
Based on the regressions being bug 655649 and bug 781476, this looks like Firefox 15 is unaffected. Is this correct?
(Assignee)

Comment 15

6 years ago
(In reply to Al Billings [:abillings] from comment #14)
> Based on the regressions being bug 655649 and bug 781476, this looks like
> Firefox 15 is unaffected. Is this correct?

Correct.
status-firefox15: --- → unaffected

Updated

6 years ago
Whiteboard: [js:p1:fx18] → [js:p1:fx18][advisory-tracking-]
Confirmed crash on 2012-9-8, nightly
Verified fixed on build 2012-10-24, 16.02
Verified fixed on build 2012-11-13, 17.0b6
Verified fixed on build 2012-11-19, 17.0esr
Verified fixed on build 2012-11-19, 18.0a2 Aurora
Verified fixed on build 2012-11-19, nightly
Status: RESOLVED → VERIFIED
status-firefox16: fixed → verified
status-firefox17: fixed → verified
Keywords: verifyme
Keywords: csec-sop, sec-high
Group: core-security
You need to log in before you can comment on or make changes to this bug.