789713 - Assertion failure: wrapper->isWrapper() setting domain

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

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

Comment 1

[Bob Clary [:bc] (inactive)]

Doh. actually this is in Beta/16, and Aurora/17 as well. Looks like Bug 781476  to me.

Comment 2

[Bobby Holley (:bholley)]

This is an awesome bug report. Thanks Bob.

Comment 3

[Bobby Holley (:bholley)]

Let's make this s-s for now.

Comment 4

[Bobby Holley (:bholley)]

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.

Comment 5

[Bobby Holley (:bholley)]

Attachment: Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. v1

Attaching a patch. Flagging mrbkap for review.

Comment 6

[bhavana bajaj [:bajaj]]

Tracking for FF16,FF17 as this blocks Bug 787637 which is being tracked as well.

Comment 7

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=e682942ddb53

Comment 8

[Bobby Holley (:bholley)]

Green on try. Pushed to m-i:

https://hg.mozilla.org/integration/mozilla-inbound/rev/132b9b646ebc

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/132b9b646ebc

Comment 10

[Alex Keybl [:akeybl]]

Please nominate for aurora/beta approval once comfortable with the changes on Nightly.

Comment 11

[Bobby Holley (:bholley)]

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

Comment 12

[Alex Keybl [:akeybl]]

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.

Comment 13

[Bobby Holley (:bholley)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3e04584e30f7
https://hg.mozilla.org/releases/mozilla-beta/rev/66c9fbb9f4d3

Comment 14

[Al Billings [:abillings - ex-MoCo]]

Based on the regressions being bug 655649 and bug 781476, this looks like Firefox 15 is unaffected. Is this correct?

Comment 15

[Bobby Holley (:bholley)]

(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.

Comment 16

[Matt Wobensmith [:mwobensmith][:matt:]]

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