430593 - crash [@ NPObjWrapper_NewResolve] when modifying object element, cloning it and inserting the clone in the document

Status: RESOLVED FIXED

(Core :: Plug-ins, defect, critical)

Description

[Michael A. Puls II]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042305 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042305 Minefield/3.0pre

If you have a parsed object element that uses type="application/x-mplayer2" that's handled by the VideoLan Mozilla plugin, modify the object element's attributes and params with JS to load a Java applet (Java SE6 update 10 beta), clone the modified object, insert the clone into the document and remove the old object, FF3 will crash.

Works fine in FF2.

Will upload TC.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Michael A. Puls II]

Attachment: Extract and load objcrash.html. It will crash FF3 and load fine in FF2.

Comment 2

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Is there a breakpad id for this crash?

Martijn, can you reproduce by any chance?

Comment 3

[Michael A. Puls II]

<http://crash-stats.mozilla.com/report/index/47437163-11dc-11dd-8bf5-001cc4e2bf68?p=1>

Does that help?

Comment 4

[Michael A. Puls II]

Attachment: Here's a slightly modified TC that actually triggers breakpad.

I just let the page fully load and if it doesn't crash right away, I just refresh.  The crash totally goes away if I get rid of the obj.style.width line.

Comment 5

[Martijn Wargers (zombie)]

http://crash-stats.mozilla.com/report/index/47437163-11dc-11dd-8bf5-001cc4e2bf68?p=1
0  	xul.dll  	NPObjWrapper_NewResolve  	 mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp:1537
1 	js3250.dll 	js_LookupPropertyWithFlags 	mozilla/js/src/jsobj.c:3320
2 	js3250.dll 	js_GetPropertyHelper 	mozilla/js/src/jsobj.c:3636
3 	js3250.dll 	js_Interpret 	mozilla/js/src/jsinterp.c:4162
4 	js3250.dll 	js_Invoke 	mozilla/js/src/jsinterp.c:1299
5 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1523
6 	xul.dll 	nsXPCWrappedJS::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp:559
7 	xul.dll 	PrepareAndDispatch 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
8 	xul.dll 	SharedStub 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
9 	xul.dll 	nsEventListenerManager::HandleEventSubType 	mozilla/content/events/src/nsEventListenerManager.cpp:1080


I crashed the first time with the slightly modified testcase, but breakpad didn't come up (I also didn't have VideoLAN plugin installed at that time).
After that I have been unable to crash.
Firefox does disappear suddenly, though, when I quickly reload a couple of times, but breakpad doesn't come up.

Using:
Videolan Version 0.8.6f
Java Plug-in 1.6.0_10

Comment 6

[Martijn Wargers (zombie)]

Attachment: stack from debug build

In a debug build, however, I crash nicely.

Comment 7

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Line 1537 in NPObjWrapper_NewResolve is:

  1537   } else if (npobj->_class->hasMethod(npobj, (NPIdentifier)id)) {

Martijn, what does npobj->_class look like in your debugger?  What about npobj->_class->hasMethod?  It sounds like npobj might be dead and us jumping off into lala-land, which is not a good idea...

Comment 8

[Martijn Wargers (zombie)]

I'm getting this in the Quickwatch:
npobj	0x0d052410 {_class=0x00000000 referenceCount=2 }	NPObject *
 _class	0x00000000 {structVersion=??? allocate=??? deallocate=??? ...}	NPClass *  
 referenceCount	2	unsigned int

For npobj->_class->hasMethod I'm getting this:
hasMethod	CXX0030: Error: expression cannot be evaluated	

Comment 9

[Boris Zbarsky [:bzbarsky, bz on IRC]]

So... npobj->_class seems to be null?  But we checked that on line 1511.  Did the npobj->_class->hasProperty() call somehow kill npobj or some such?  jst, is that possible?

Comment 10

[Damon Sicore (:damons)]

Is this use case popular?  Why should it block?  If this were the last bug, would you hold the release for it?  Could we take it in a dot release?  Please re-nom when answered. :)

Comment 11

[Boris Zbarsky [:bzbarsky, bz on IRC]]

> Is this use case popular? 

Unclear, since we don't know how we get here.  This code _is_ going to be exercised more with the new Java plug-in, though.

> Why should it block? 

It looks potentially exploitable.  It should get investigated.  It's not clear whether it should block.

> If this were the last bug, would you hold the release for it?

I'd try to take the time to investigate it, at least.  That way I'd actually be able to make informed decisions on these questions.  If the investigation took to long, I'd go ahead and ship, and make this a priority to fix in the first dot release.

> Could we take it in a dot release?

It depends on what's going wrong, but I'd certainly hope so.  If we don't take it for final, we should definitely do this.

I guess I'll nominate this for a dot release, but it really just needs jst to look at it.

Comment 12

[Boris Zbarsky [:bzbarsky, bz on IRC]]

If the question is whether the clone-and-insert thing is popular, it's the only way to reliably "reload" a plug-in cross-browser.

Comment 13

[Martijn Wargers (zombie)]

This should be fixed now by bug 486269.