604368 - Sandbox.evalInSandbox() crash [@ XPCWrappedNative::GetObjectPrincipal ] [@ nsXPConnect::GetPrincipal(JSObject*, int)]

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Scoobidiver (away)]

Build: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101014
Firefox/4.0b8pre

This is a new crash signature. Crashes first appeared in b8pre/20101011 build.
It happens on Mac OS X and linux.
It is #7 top crasher in 4.0b8pre for the last week.

Signature	XPCWrappedNative::GetObjectPrincipal
UUID	b44377e0-5c92-454a-a75b-3f1792101014
Time 	2010-10-14 07:35:55.20714
Uptime	684
Last Crash	686 seconds (11.4 minutes) before submission
Install Age	1657 seconds (27.6 minutes) since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101013214358
Branch	2.0
OS	Mac OS X
OS Version	10.6.5 10H542
CPU	amd64
CPU Info	family 6 model 15 stepping 11
Crash Reason	EXC_BAD_ACCESS / 0x0000000d
Crash Address	0x0
App Notes 	Renderers: 0x22600,0x20400

Frame 	Module 	Signature [Expand] 	Source
0 	XUL 	XPCWrappedNative::GetObjectPrincipal 	js/src/xpconnect/src/xpcprivate.h:1465
1 	XUL 	nsXPConnect::GetPrincipal 	js/src/xpconnect/src/nsXPConnect.cpp:2442
2 	XUL 	xpc::Transparent 	js/src/xpconnect/wrappers/XrayWrapper.cpp:444
3 	XUL 	xpc::XrayWrapper<JSCrossCompartmentWrapper,xpc::CrossCompartmentXray>::getPropertyDescriptor 	js/src/xpconnect/wrappers/XrayWrapper.cpp:531
4 	XUL 	js::JSProxyHandler::has 	js/src/jsproxy.cpp:96
5 	XUL 	xpc::XrayWrapper<JSCrossCompartmentWrapper,xpc::CrossCompartmentXray>::has 	js/src/xpconnect/wrappers/XrayWrapper.cpp:782
6 	XUL 	js::proxy_LookupProperty 	js/src/jsproxy.cpp:760
7 	XUL 	js_LookupPropertyWithFlags 	js/src/jsobj.h:1034
8 	XUL 	js_FindPropertyHelper 	js/src/jsobj.cpp:4745
9 	XUL 	js::Interpret 	js/src/jsinterp.cpp:4819
10 	XUL 	js::Execute 	js/src/jsinterp.cpp:638
11 	XUL 	JS_EvaluateUCScriptForPrincipals 	js/src/jsapi.cpp:4882
12 	XUL 	xpc_EvalInSandbox 	js/src/xpconnect/src/xpccomponents.cpp:3661
13 	XUL 	nsXPCComponents_Utils::EvalInSandbox 	js/src/xpconnect/src/xpccomponents.cpp:3551
14 	XUL 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
15 	XUL 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3054
16 	XUL 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1631
17 	XUL 	js::Interpret 	js/src/jscntxtinlines.h:652
...

The regression range is :
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=26c47ba8064f&tochange=5a41a70eb631

More reports at:
http://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=XPCWrappedNative%3A%3AGetObjectPrincipal

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

cc'ing roc and mats who are suspect in that regression range

I hit this a couple of times this morning, both times on osx64, seemed to be related to this URL: http://rypp.ly/bHk8Lk (couldn't get other people to reproduce it, though)

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Actually, turns out that my crash is slightly different, but related. Mine looks like this:

http://crash-stats.mozilla.com/report/index/bp-0f4cb3d7-1467-4829-864f-dd36b2101014

Comment 3

[Andreas Gal :gal]

I think this is a dup of bug 544610.

Comment 5

[Giorgio Maone [:ma1]]

This doesn't seem a dupe, since current stacks consistently contain an Sandbox.evalInSandbox() call and compartments definitely mess with Sandbox, as seen in bug 604346

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Can't get it to happen in Safe Mode, which implies one of my add-ons; more as I know it!

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

I believe that one of Account Manager or Contacts was causing the problem; Hanson/Mills, can you take a look at the stack and see if that's a possibility?

Comment 8

[Giorgio Maone [:ma1]]

(In reply to comment #7)
> I believe that one of Account Manager or Contacts was causing the problem;
> Hanson/Mills, can you take a look at the stack and see if that's a possibility?

NoScript is reported to cause that, even though I didn't manage to reproduce, and looking at the stacks (which contain both evalInSandbox() and notifyObservers()) I'm almost sure it's the Script Surrogate feature, which executes scripts in a Sandbox with the content window principal in a content-document-global-created observer.

Comment 9

[Scoobidiver (away)]

From b8pre/20101011 build, crashes on Windows with nsXPConnect::GetPrincipal(JSObject*, int) as a crash signature have also a stack trace that contains Sandbox.evalInSandbox() call.

Comment 10

[Michael Hanson]

Contacts does use evalInSandbox() but we haven't released a version that supports 4.0b* - unless they're running a prerelease users are unlikely to be using Contacts on 4.0.

Comment 11

[Andreas Gal :gal]

This is likely related to a evalInSandbox API change.

Comment 12

[Giorgio Maone [:ma1]]

(In reply to comment #11)
> This is likely related to a evalInSandbox API change.

Which change, exactly?

Comment 13

[Dan Mills [:thunder]]

Account Manager does not use evalInSandbox(), but it does (in the addon version, not the patch) call ProcessNextEvent to simulate synchronous functions.  It does so in the middle of a request handler (onDataAvailable and such).

Comment 14

[Andreas Gal :gal]

In the past you could create a sandbox and then set its __proto__ to another object. You know have to request the object to be assigned that proto at sandbox creation time. Also, you can pick the wrapper you get (xray or not) with an optional parameter.

Comment 15

[Giorgio Maone [:ma1]]

(In reply to comment #14)
> In the past you could create a sandbox and then set its __proto__ to another
> object. You know have to request the object to be assigned that proto at
> sandbox creation time. Also, you can pick the wrapper you get (xray or not)
> with an optional parameter.

Where are these changes documented? I couldn't find anything at http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/xpccomponents.idl

At any rate, NoScript is not replacing __proto__ nor is picking any wrapper.

The code is:

    var w = document.defaultView;
    try {
      var s = new CU.Sandbox(w);
      s.window = (typeof w.wrappedJSObject == "object") ? w.wrappedJSObject : w;
      CU.evalInSandbox("with(window) {" + scriptBlock + "}", s);
    } catch(e) {
      if (ns.consoleDump) ns.dump(e);
      if (this.debug) CU.reportError(e);
    }

where document is a content document and scriptBlock is a configurable but trusted script source.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #10)
> Contacts does use evalInSandbox() but we haven't released a version that
> supports 4.0b* - unless they're running a prerelease users are unlikely to be
> using Contacts on 4.0.

Yeah, I was running it and Account Manager with the Add-on Compatibility reporter forcing compatibility on. I guess the pages I crashed on were tickling those Add-ons. I suspect that this topcrasher might be heavily correlated with other Add-ons that do the same.

Gal: you've said this is related to a change, but I can't tell if you're saying that it's a bug from that change you intend to fix, or if we need to make sure Add-on authors know. Either way, a straight-out crash isn't a great outcome :(

Comment 19

[Daniel Holbert [:dholbert]]

FWIW, I hit this crash pretty much every time I load any URL on twitter.com, logged in (and using the NewTwitter preview). e.g. https://twitter.com/mozillagary triggered it for me.

I was able to reproduce in reliably in a fresh profile, with Noscript development version installed, set to allow all scripts on the page (on a per-script-domain basis, using the icon in the Addon Toolbar).  (The final domain that I allowed before it started crashing was google-analytics.com)

Comment 20

[Daniel Holbert [:dholbert]]

Adding 'dogfood' keyword, as this crash makes today's nightly hard to dogfood when viewing popular sites like twitter that trigger it. :(

Comment 21

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #20)
> Adding 'dogfood' keyword, as this crash makes today's nightly hard to dogfood
> when viewing popular sites like twitter that trigger it. :(

Works in safe mode, doesn't it? That implies that it's a dogfood issue for some of the add-ons.

Comment 22

[Daniel Holbert [:dholbert]]

It's a dogfood issue for those who like to run Firefox with NoScript (myself included).

Comment 23

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

The bug here is that NoScript creates a sandbox object around a window that has document.domain set and we aren't prepared to handle that. This fixes the bug without slowing down nsXPConnect::GetPrincipal for the normal case.

Comment 26

[Laxminarayan G Kamath A]

After  http://crash-stats.mozilla.com/report/index/bp-c4152c9e-191d-4951-a044-c3f812101014 , I moved greasemonkey out of the zoo and firefox became sane again.

Comment 27

[Giorgio Maone [:ma1]]

(In reply to comment #23)
> Created attachment 483344 [details] [diff] [review]
> Fix
> 
> The bug here is that NoScript creates a sandbox object around a window that has
> document.domain set and we aren't prepared to handle that.

Do you mean document.domain set by some content script to something different than document.documentURI's host?
If so, how can it be possible during a content-document-global-created notification, when no content script has been ran yet?

Comment 29

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #27)
> Do you mean document.domain set by some content script to something different
> than document.documentURI's host?
> If so, how can it be possible during a content-document-global-created
> notification, when no content script has been ran yet?

document.domain actually mutates the principals of the page that it's set on, and then the behavior derives from that. So, when the sandbox runs, we end up behaving like document.domain has been set, which the code wasn't prepared to do in a sandbox.

Comment 30

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/72d1bbea09ed

Comment 31

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/c66660664ec7

Comment 32

[Myzar]

I still crash with http://hg.mozilla.org/mozilla-central/rev/c66660664ec7

and noscript

Comment 33

[Giorgio Maone [:ma1]]

(In reply to comment #32)
> I still crash with http://hg.mozilla.org/mozilla-central/rev/c66660664ec7
> 
> and noscript

Stack?

Comment 34

[Myzar]

http://crash-stats.mozilla.com/report/index/bp-4481444c-cc99-4870-937e-3f1482101016

no symbols the nightly isn't out yet

Comment 35

[Myzar]

new stack with symbols

http://crash-stats.mozilla.com/report/index/29dd17e5-85e9-4de7-9791-387012101016

Comment 36

[Giorgio Maone [:ma1]]

(In reply to comment #35)
> new stack with symbols
> 
> http://crash-stats.mozilla.com/report/index/29dd17e5-85e9-4de7-9791-387012101016

Looks like the same bug, indeed, or the same call chain at least.

Comment 37

[WildcatRay]

With build "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101016 Firefox/4.0b8pre ID:20101016041245", I get the following crash:

http://crash-stats.mozilla.com/report/index/450ba00e-4308-4a60-b1a1-b4fd12101016

when trying to open http://www.wunderground.com/cgi-bin/findweather/getForecast?query=43147. However, it may be add-on related as the page does not crash the browser in a fairly new, no installed add-ons profile. My main profile does not have either ABP or NoScript installed.

I am providing the crash ID in case it helps to troubleshoot this.

Comment 38

[Giorgio Maone [:ma1]]

(In reply to comment #37)
> However, it may be add-on related as the page does not crash the browser in a
> fairly new, no installed add-ons profile. My main profile does not have either
> ABP or NoScript installed.
> 
It's GreaseMonkey, in your case.

Comment 39

[WildcatRay]

(In reply to comment #38)
> (In reply to comment #37)
> > However, it may be add-on related as the page does not crash the browser in a
> > fairly new, no installed add-ons profile. My main profile does not have either
> > ABP or NoScript installed.
> > 
> It's GreaseMonkey, in your case.

After trying the disable/enable troubleshooting, I discovered I had to clear the browser cache, too, to get the crash.

After disabling GM, the crashes have stopped. My GM version is the 2010.10.14beta. I need to see if the new nightly has been generated yet.

Comment 40

[WildcatRay]

(In reply to comment #39)
> (In reply to comment #38)
> > (In reply to comment #37)
> > > However, it may be add-on related as the page does not crash the browser in a
> > > fairly new, no installed add-ons profile. My main profile does not have either
> > > ABP or NoScript installed.
> > > 
> > It's GreaseMonkey, in your case.
> 
> After trying the disable/enable troubleshooting, I discovered I had to clear
> the browser cache, too, to get the crash.
> 
> After disabling GM, the crashes have stopped. My GM version is the
> 2010.10.14beta. I need to see if the new nightly has been generated yet.

Crash even with the 2010.10.15beta version of GM.

Other crashes:
http://crash-stats.mozilla.com/report/index/bp-029ac9e9-4049-4af7-a07a-6b7142101016
http://crash-stats.mozilla.com/report/index/bp-f13f7123-2416-41f1-a067-a94972101016
http://crash-stats.mozilla.com/report/index/bp-c743688e-87cd-48f6-9097-4c3832101016
http://crash-stats.mozilla.com/report/index/bp-dea834c8-28e1-4091-ae44-652322101016
http://crash-stats.mozilla.com/report/index/bp-62dc7d6e-e570-4cbe-82a6-f30802101016

Comment 41

[Daniel Holbert [:dholbert]]

The STR in comment 19 (with Twitter) still repro this for me, too:
Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101016 Firefox/4.0b8pre
bp-552314d2-17d0-411b-9a71-0b6932101016

Comment 42

[Andreas Jung]

Using Greasemonkey[1] and Autopagerize[2] I still get the crash on 
Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101016 Firefox/4.0b8pre,
but the stack is different:
http://crash-stats.mozilla.com/report/index/bp-05dc4092-f88f-4da2-96cc-c5f672101016
http://crash-stats.mozilla.com/report/index/bp-598c599b-1f64-4fb6-a7fb-2b0652101016

Same Bug? Different Bug?

[1] https://arantius.com/misc/gm-nightly/greasemonkey-2010.10.15.beta.xpi
[2] http://userscripts.org/scripts/show/8551

Comment 44

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Followup

I didn't realize the code fixed here had been copied. This shares the two copies of the code (and makes the fix apply to the copied bit in the process).

Comment 45

[Brendan Eich [:brendan]]

Code copying considered harmful (buckets of it in methodjit from jsinterp.cpp too). Avoid.

/be

Comment 47

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/mozilla-central/rev/22028fb65bb2