Last Comment Bug 441714 - Crash [@ JS_GetFunctionScript ] with setTimeout and XPCSafeJSObjectWrapper
: Crash [@ JS_GetFunctionScript ] with setTimeout and XPCSafeJSObjectWrapper
Status: RESOLVED FIXED
[sg:investigate]
: crash, regression, testcase, verified1.9.0.14, verified1.9.1
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
http://www.websudoku.com/
Depends on:
Blocks: 425139
  Show dependency treegraph
 
Reported: 2008-06-24 19:21 PDT by Taral
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
dveditz: wanted1.9.1.x+
dveditz: blocking1.9.0.14+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2+
.2-fixed


Attachments
testcase (crashes Firefox when loaded) (95 bytes, text/html)
2008-07-04 04:57 PDT, moz_bug_r_a4
no flags Details
Spot fix (1.10 KB, patch)
2009-06-04 16:35 PDT, Blake Kaplan (:mrbkap)
dveditz: review+
dveditz: superreview+
mbeltzner: approval1.9.1.2+
dveditz: approval1.9.0.14+
Details | Diff | Splinter Review

Description Taral 2008-06-24 19:21:27 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

Based on the crash data, it looks like the calls to setTimeout are causing Firefox to crash.

Reproducible: Always

Steps to Reproduce:
1. Install Greasemonkey 0.8.20080609.0
2. Install script: http://taral.dreamhost.com/websudokuig.user.js
3. Visit http://www.websudoku.com/
4. Enable pencil marking in options
5. Hit the update button
Actual Results:  
Crash! http://crash-stats.mozilla.com/report/index/18026f7a-425b-11dd-b225-001cc45a2c28

Expected Results:  
Not crash
Comment 1 moz_bug_r_a4 2008-07-04 04:57:39 PDT
Created attachment 328109 [details]
testcase (crashes Firefox when loaded)

This does not require Greasemonkey.
Comment 2 moz_bug_r_a4 2008-07-04 04:58:57 PDT
This is a regression from bug 425139.

Due to the fix for bug 425139, setTimeout() treats an XPCSafeJSObjectWrapper
wrapping a function as a function.  But, nsJSContext::CallEventHandler does not
handle the XPCSafeJSObjectWrapper properly.


Breakpoint 1, nsJSContext::CallEventHandler (this=0xb0060240, 
    aTarget=0xb643e5c0, aScope=0xb5511e80, aHandler=0xb2c55f00, 
    aargv=0xafd3f644, arv=0xbf9e96bc) at nsJSEnvironment.cpp:1876
1876      NS_ENSURE_TRUE(mIsInitialized, NS_ERROR_NOT_INITIALIZED);
(gdb) p *(JSClass*)(((JSObject*)aHandler)->fslots[2]&~1)
$1 = {name = 0xb63aa6e0 "XPCSafeJSObjectWrapper", flags = 66820, 
  addProperty = 0xb6387a3e <XPC_SJOW_AddProperty>, 
  delProperty = 0xb63879c0 <XPC_SJOW_DelProperty>, 
  getProperty = 0xb6387776 <XPC_SJOW_GetProperty>, 
  setProperty = 0xb6387752 <XPC_SJOW_SetProperty>, 
  enumerate = 0xb638790e <XPC_SJOW_Enumerate>, 
  resolve = 0xb6387426 <XPC_SJOW_NewResolve>, 
  convert = 0xb6385fdc <XPC_SJOW_Convert>, 
  finalize = 0xb6386898 <XPC_SJOW_Finalize>, getObjectOps = 0, 
  checkAccess = 0xb6386068 <XPC_SJOW_CheckAccess>, 
  call = 0xb6387076 <XPC_SJOW_Call>, 
  construct = 0xb6387db4 <XPC_SJOW_Construct(JSContext*, JSObject*, unsigned int, long*, long*)>, xdrObject = 0, hasInstance = 0, mark = 0, reserveSlots = 0}
(gdb) n
1878      if (!mScriptsEnabled) {
(gdb) 
1883      JSObject* target = nsnull;
(gdb) 
1884      nsAutoGCRoot root(&target, &rv);
(gdb) 
1885      NS_ENSURE_SUCCESS(rv, rv);
(gdb) 
1886      rv = JSObjectFromInterface(aTarget, aScope, &target);
(gdb) 
1887      NS_ENSURE_SUCCESS(rv, rv);
(gdb) 
1889      jsval rval = JSVAL_VOID;
(gdb) 
1897        do_GetService("@mozilla.org/js/xpc/ContextStack;1", &rv);
(gdb) 
1898      if (NS_FAILED(rv) || NS_FAILED(stack->Push(mContext)))
(gdb) 
1902      rv = sSecurityManager->CheckFunctionAccess(mContext, aHandler, target);
(gdb) s
nsScriptSecurityManager::CheckFunctionAccess (this=0xb6dcf1a0, aCx=0xb067f0a0, 
    aFunObj=0xb2c55f00, aTargetObj=0xb5511e80)
    at nsScriptSecurityManager.cpp:1652
1652            GetFunctionObjectPrincipal(aCx, (JSObject *)aFunObj, nsnull, &rv);
(gdb) 
nsScriptSecurityManager::GetFunctionObjectPrincipal (cx=0xb067f0a0, 
    obj=0xb2c55f00, fp=0x0, rv=0xbf9e94c0) at nsScriptSecurityManager.cpp:2174
2174        NS_PRECONDITION(rv, "Null out param");
(gdb) 
2175        JSFunction *fun = (JSFunction *) caps_GetJSPrivate(obj);
(gdb) 
caps_GetJSPrivate (obj=0xb2c55f00) at nsScriptSecurityManager.cpp:138
138         JS_ASSERT(STOBJ_GET_CLASS(obj)->flags & JSCLASS_HAS_PRIVATE);
(gdb) p *(JSClass*)(obj->fslots[2]&~1)
$2 = {name = 0xb63aa6e0 "XPCSafeJSObjectWrapper", flags = 66820, 
  addProperty = 0xb6387a3e <XPC_SJOW_AddProperty>, 
  delProperty = 0xb63879c0 <XPC_SJOW_DelProperty>, 
  getProperty = 0xb6387776 <XPC_SJOW_GetProperty>, 
  setProperty = 0xb6387752 <XPC_SJOW_SetProperty>, 
  enumerate = 0xb638790e <XPC_SJOW_Enumerate>, 
  resolve = 0xb6387426 <XPC_SJOW_NewResolve>, 
  convert = 0xb6385fdc <XPC_SJOW_Convert>, 
  finalize = 0xb6386898 <XPC_SJOW_Finalize>, getObjectOps = 0, 
  checkAccess = 0xb6386068 <XPC_SJOW_CheckAccess>, 
  call = 0xb6387076 <XPC_SJOW_Call>, 
  construct = 0xb6387db4 <XPC_SJOW_Construct(JSContext*, JSObject*, unsigned int, long*, long*)>, xdrObject = 0, hasInstance = 0, mark = 0, reserveSlots = 0}
(gdb) n

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0xb566e850 "STOBJ_GET_CLASS(obj)->flags & JSCLASS_HAS_PRIVATE", 
    file=0xb566e031 "nsScriptSecurityManager.cpp", ln=138) at jsutil.cpp:63
63          abort();
(gdb) ret
Make JS_Assert return now? (y or n) y
#0  caps_GetJSPrivate (obj=0xb2c55f00)
    at nsScriptSecurityManager.cpp:139
139         v = obj->fslots[JSSLOT_PRIVATE];
(gdb) n
140         if (!JSVAL_IS_INT(v))
(gdb) p v
$3 = 6
(gdb) n
141             return NULL;
(gdb) 
143     }
(gdb) 
nsScriptSecurityManager::GetFunctionObjectPrincipal (cx=0xb067f0a0, 
    obj=0xb2c55f00, fp=0x0, rv=0xbf9e94c0) at nsScriptSecurityManager.cpp:2176
2176        JSScript *script = JS_GetFunctionScript(cx, fun);
(gdb) p fun
$4 = (JSFunction *) 0x0
(gdb) n

Program received signal SIGSEGV, Segmentation fault.
0xb7e6d1f7 in JS_GetFunctionScript (cx=0xb067f0a0, fun=0x0) at jsdbgapi.cpp:948
948         return FUN_SCRIPT(fun);
(gdb) 
Comment 3 Daniel Veditz [:dveditz] 2009-05-21 10:32:11 PDT
Still crashes 1.9.0.x: bp-f68877f6-3537-4e1f-a85c-b37ee2090521
and 1.9.1: bp-b125b377-7e34-425c-b61b-4cce02090521

This testcase crashes because fun is null so it might be safe enough, but marking as a security bug for investigation. JS_Asserts are usually bad news and mishandled wrappers are worrying too.

Moving this to the right component from the Firefox:General "lost and found" bin.
Comment 4 Blake Kaplan (:mrbkap) 2009-05-21 11:05:49 PDT
I'll take this. It's my fault.
Comment 5 Blake Kaplan (:mrbkap) 2009-06-04 16:35:50 PDT
Created attachment 381648 [details] [diff] [review]
Spot fix

Amazingly, this is the only spot in our codebase that makes the JSTYPE_FUNCTION -> JS_ObjectIsFunction connection, so protecting against it here is sufficient.
Comment 6 Daniel Veditz [:dveditz] 2009-06-07 10:39:13 PDT
We should fix this on 1.9.0.x and 1.9.1.x as well (though unless we can get a non-null crash no need to try and catch the 3.5 train that's already pulling out of the station).
Comment 7 Daniel Veditz [:dveditz] 2009-06-07 10:39:26 PDT
Comment on attachment 381648 [details] [diff] [review]
Spot fix

r/sr=dveditz
Comment 8 Blake Kaplan (:mrbkap) 2009-06-12 14:54:45 PDT
http://hg.mozilla.org/mozilla-central/rev/cccc5f021924
Comment 9 Seth Spitzer 2009-06-29 09:51:52 PDT
bkap, dveditz and josh:  could changes for bug #486574 (which has made objective c exceptions non-fatal) be causing us to hit this more often, and with non-null crashes?

for example, see http://crash-stats.mozilla.com/report/index/9d98c4be-c6d1-44b6-a9fe-283f82090628

The App Notes for that crash are:  "Obj-C Exception data:
NSInvalidArgumentException: *** -[NSCFType setStringValue:]: unrecognized selector sent to instance 0x16803c60"

Before the fix for bug #486574, this crash would have show up as 
crash @ nsObjCExceptionLogAbort (like bug #500470).

In Postbox, we had a crash (like bug #500470).  After applying the patch for bug #486574, we're now seeing a crash that might be fixed by this bug.  (We're still investigating.)
Comment 10 Seth Spitzer 2009-06-29 09:57:36 PDT
in case it helps, here's a link to all the fx crashes containing JS_GetFunctionScript() in the past week:  http://bit.ly/AW96a
Comment 11 Samuel Sidler (old account; do not CC) 2009-07-13 14:48:54 PDT
Not blocking 1.9.1.1, but definitely for 1.9.1.2.

Any answer to Seth's questions in comment 9?
Comment 12 Blake Kaplan (:mrbkap) 2009-07-14 15:56:33 PDT
I don't think that the two patches are related in any way. This patch might be allowing Seth to get lucky, though.
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2009-07-21 17:14:56 PDT
We'd take a reviewed patch with an approval1.9.1.2? request, most likely, but if it's not ready in time for 1.9.1.2 (end of July) we'll wait until 1.9.1.3.
Comment 14 Blake Kaplan (:mrbkap) 2009-07-22 12:43:15 PDT
Comment on attachment 381648 [details] [diff] [review]
Spot fix

This applies as-is.
Comment 15 Mike Beltzner [:beltzner, not reading bugmail] 2009-07-22 14:52:15 PDT
Comment on attachment 381648 [details] [diff] [review]
Spot fix

a1912=beltzner, please land on mozilla-1.9.1
Comment 16 Blake Kaplan (:mrbkap) 2009-07-22 17:52:56 PDT
Fixed in http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0f45c73bf86a (is this the right way to mark it?!)
Comment 17 Samuel Sidler (old account; do not CC) 2009-07-22 20:14:41 PDT
(In reply to comment #16)
> (is this the right way to mark it?!)

(yes)
Comment 18 juan becerra [:juanb] 2009-07-30 20:34:18 PDT
Verified using test case in comment #1. 3.5 crashes on load. 3.5.2 does not.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Comment 19 Daniel Veditz [:dveditz] 2009-08-05 16:30:45 PDT
blake: does this patch work for 1.9.0 or do we need a separate patch? Will take your approval1.9.0.14 request as the answer :-)
Comment 20 Daniel Veditz [:dveditz] 2009-08-07 11:18:19 PDT
Comment on attachment 381648 [details] [diff] [review]
Spot fix

Approved for 1.9.0.14, a=dveditz for release-drivers
Comment 21 Blake Kaplan (:mrbkap) 2009-08-10 17:13:10 PDT
Checking in caps/src/nsScriptSecurityManager.cpp;
/cvsroot/mozilla/caps/src/nsScriptSecurityManager.cpp,v  <--  nsScriptSecurityManager.cpp
new revision: 1.365; previous revision: 1.364
done
Comment 22 Al Billings [:abillings] 2009-08-18 16:52:06 PDT
Verified fixed for 1.9.0.14 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.14pre) Gecko/2009081305 GranParadiso/3.0.14pre (.NET CLR 3.5.30729).

Note You need to log in before you can comment on or make changes to this bug.