289645 - nsXBLPrototypeHandler::ExecuteHandler doesn't null/rv check

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[timeless]

/* this failed: */
    boundContext->CompileEventHandler(scriptObject, onEventAtom, eventName,
                                      handlerText,
                                      bindingURI.get(),
                                      mLineNumber,
                                      PR_TRUE, &handler);
/* but its return was not stored, nor is handler null checked */
  }

  // Temporarily bind it to the bound element
/* this succeeds (binding null): */
  boundContext->BindCompiledEventHandler(scriptObject, onEventAtom, handler);

/* this can fail:
>	gklayout.dll!NS_NewJSEventListener(nsIDOMEventListener * *
aInstancePtrResult=0x0012e988, nsIScriptContext * aContext=0x026134b8,
nsISupports * aObject=0x18e9c298)  Line 224	C++
 	gklayout.dll!nsDOMScriptObjectFactory::NewJSEventListener(nsIScriptContext *
aContext=0x026134b8, nsISupports * aObject=0x18e9c298, nsIDOMEventListener * *
aInstancePtrResult=0x0012e988)  Line 111 + 0x11	C++
 	gklayout.dll!nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventReceiver *
aReceiver=0x18e9c298, nsIDOMEvent * aEvent=0x0229b548)  Line 486	C++
 	gklayout.dll!nsXBLEventHandler::HandleEvent(nsIDOMEvent * aEvent=0x18e9c298) 
Line 85	C++
 	gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct *
aListenerStruct=0x010e3bfc, nsIDOMEvent * aDOMEvent=0x0012e9ec,
nsIDOMEventTarget * aCurrentTarget=0x0102e903, unsigned int aSubType=0x18e9c298,
unsigned int aPhaseFlags=0x0229b548)  Line 1512 + 0xb	C++
*/
  factory->NewJSEventListener(boundContext, aReceiver,
                              getter_AddRefs(eventListener));

/* when that fails, this will be null: */
  nsCOMPtr<nsIJSEventListener> jsListener(do_QueryInterface(eventListener));
/* and this will crash! */
  jsListener->SetEventName(onEventAtom);
/* and if that didn't crash, this would! */
  eventListener->HandleEvent(aEvent);

 	js3250.dll!js_DefineNativeProperty(JSContext * cx=0x02601940, JSObject *
obj=0x025dec78, long id=0x02481c68, long value=0x00000000, int (JSContext *,
JSObject *, long, long *)* getter=0x00ad1b31, int (JSContext *, JSObject *,
long, long *)* setter=0x00000000, unsigned int attrs=0x00000005, unsigned int
flags=0x00000000, int shortid=0x00000000, JSProperty * * propp=0x00000000) 
Line 2336	C
 	js3250.dll!js_DefineProperty(JSContext * cx=0x02601940, JSObject *
obj=0x025dec78, long id=0x02481c68, long value=0x00000000, int (JSContext *,
JSObject *, long, long *)* getter=0x00000000, int (JSContext *, JSObject *,
long, long *)* setter=0x00000000, unsigned int attrs=0x00000005, JSProperty * *
propp=0x00000000)  Line 2246 + 0x21	C
 	js3250.dll!DefineProperty(JSContext * cx=0x02601940, JSObject *
obj=0x025dec78, const char * name=0x000b0023, long value=0x00000000, int
(JSContext *, JSObject *, long, long *)* getter=0x00000000, int (JSContext *,
JSObject *, long, long *)* setter=0x00000000, unsigned int attrs=0x025f66f0,
unsigned int flags=0x00000000, int tinyid=0x00000000)  Line 2275 + 0x18	C
 	js3250.dll!JS_DefineProperty(JSContext * cx=0x02601940, JSObject *
obj=0x025dec78, const char * name=0x02576878, long value=0x00000000, int
(JSContext *, JSObject *, long, long *)* getter=0x00000000, int (JSContext *,
JSObject *, long, long *)* setter=0x00000000, unsigned int attrs=0x00000005) 
Line 2363 + 0x20	C
 	gklayout.dll!nsJSContext::BindCompiledEventHandler(void * aTarget=0x025dec78,
nsIAtom * aName=0x02576878, void * aHandler=0x00000000)  Line 1387 + 0x16	C++
>	gklayout.dll!nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventReceiver *
aReceiver=0x00000000, nsIDOMEvent * aEvent=0x00000000)  Line 478	C++
 	gklayout.dll!nsXBLEventHandler::HandleEvent(nsIDOMEvent * aEvent=0x18e9c298) 
Line 85	C++
 	gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct *
aListenerStruct=0x00000000, nsIDOMEvent * aDOMEvent=0x00000000,
nsIDOMEventTarget * aCurrentTarget=0x00000000, unsigned int aSubType=0x00000000,
unsigned int aPhaseFlags=0x00000000)  Line 1512 + 0xb	C++
 	gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext *
aPresContext=0x00000000, nsEvent * aEvent=0x0012ef5c, nsIDOMEvent * *
aDOMEvent=0x0012edb8, nsIDOMEventTarget * aCurrentTarget=0x18e9c298, unsigned
int aFlags=0x00000007, nsEventStatus * aEventStatus=0x0012f050)  Line 1589	C++
 	gklayout.dll!nsXULElement::HandleDOMEvent(nsPresContext *
aPresContext=0x00000000, nsEvent * aEvent=0x00000000, nsIDOMEvent * *
aDOMEvent=0x00000000, unsigned int aFlags=0x00000000, nsEventStatus *
aEventStatus=0x00000000)  Line 2820	C++
 	gklayout.dll!PresShell::HandleDOMEventWithTarget(nsIContent *
aTargetContent=0x0260dcd0, nsEvent * aEvent=0x0012ef5c, nsEventStatus *
aStatus=0x0012f050)  Line 6036	C++
 	gklayout.dll!nsMenuFrame::OnCreate()  Line 1659	C++
 	gklayout.dll!nsMenuFrame::OpenMenuInternal(int aActivateFlag=0x00000000)  Line
737 + 0x5	C++
 	gklayout.dll!nsMenuFrame::AttributeChanged(nsPresContext *
aPresContext=0x025da528, nsIContent * aChild=0x02574cf8, int
aNameSpaceID=0x00000000, nsIAtom * aAttribute=0x00a5b9e0, int
aModType=0x00000002)  Line 689	C++
 	gklayout.dll!nsCSSFrameConstructor::AttributeChanged(nsPresContext *
aPresContext=0x025da528, nsIContent * aContent=0x02574cf8, int
aNameSpaceID=0x00000000, nsIAtom * aAttribute=0x00a5b9e0, int
aModType=0x00000002)  Line 10335 + 0x16	C++
 	gklayout.dll!PresShell::AttributeChanged(nsIDocument * aDocument=0x025e2f20,
nsIContent * aContent=0x02574cf8, int aNameSpaceID=0x00000000, nsIAtom *
aAttribute=0x00a5b9e0, int aModType=0x00000002)  Line 5100	C++
 	gklayout.dll!nsXULDocument::AttributeChanged(nsIContent * aElement=0x00000000,
int aNameSpaceID=0x00000000, nsIAtom * aAttribute=0x00000000, int
aModType=0x00000000)  Line 1136 + 0x14	C++
 	gklayout.dll!nsXULElement::SetAttrAndNotify(int aNamespaceID=0x00ad20ee,
nsIAtom * aAttribute=0x02601940, nsIAtom * aPrefix=0x00000000, const nsAString &
aOldValue={...}, nsAttrValue & aParsedValue={...}, int aModification=0x00000000,
int aFireMutation=0x00000000, int aNotify=0x00000000)  Line 2202	C++
 	gklayout.dll!nsXULElement::SetAttr(int aNamespaceID=0x00000000, nsIAtom *
aName=0x00000000, nsIAtom * aPrefix=0x00000000, const nsAString & aValue={...},
int aNotify=0x00000000)  Line 2125 + 0x1f	C++
 	gklayout.dll!nsXULElement::SetAttribute(const nsAString & aName={...}, const
nsAString & aValue={...})  Line 1025 + 0xf	C++
 	gklayout.dll!nsMenuFrame::OpenMenu(int aActivateFlag=0x00000001)  Line 720 +
0x40	C++
 	gklayout.dll!nsMenuFrame::ToggleMenuState()  Line 532	C++
 	gklayout.dll!nsMenuFrame::HandleEvent(nsPresContext * aPresContext=0x025da528,
nsGUIEvent * aEvent=0x00000001, nsEventStatus * aEventStatus=0x0012f8f4)  Line
394	C++
 	gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012f9ac,
nsIView * aView=0x024c9ee8, unsigned int aFlags=0x00000001, nsEventStatus *
aStatus=0x0012f8f4)  Line 6001 + 0x13	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x024c9ee8, nsGUIEvent *
aEvent=0x0012f9ac, nsEventStatus * aEventStatus=0x0012f8f4, int
aForceHandle=0x025c04f8, int & aHandled=0x01100e58)  Line 5812 + 0x11	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x00000000, nsGUIEvent
* aEvent=0x00000000, int aCaptured=0x00000000)  Line 2402	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x3d888889,
nsEventStatus * aStatus=0x0012f968)  Line 2127 + 0x14	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f9ac)  Line 166	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f9ac,
nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1074 + 0x3	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x00000000) 
Line 1095	C++
 	gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=0x00000000,
unsigned int wParam=0x00000000, nsPoint * aPoint=0x00000000)  Line 5329	C++
 	gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int
aEventType=0x0000012e, unsigned int wParam=0x00000001, nsPoint *
aPoint=0x00000000)  Line 5580 + 0x13	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=0x00000201, unsigned
int wParam=0x00000001, long lParam=0x0006003a, long * aRetValue=0x0012fd10) 
Line 4090 + 0x11	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000702f4, unsigned int
msg=0x00000201, unsigned int wParam=0x00000001, long lParam=0x025d488c)  Line
1355 + 0x10	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x28	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7	
 	user32.dll!_DispatchMessageWorker@8()  + 0xdc	
 	user32.dll!_DispatchMessageW@4()  + 0xf	
 	gkwidget.dll!nsAppShell::Run()  Line 159	C++
 	appcomps.dll!NSGetModule()  + 0x2d18b	
 	mozilla.exe!main(int argc=0x00000002, char * * argv=0x002c44f0)  Line 1813 +
0x13	C++
 	mozilla.exe!WinMain(HINSTANCE__ * __formal=0x00400000, HINSTANCE__ *
__formal=0x00400000, char * args=0x00172365, HINSTANCE__ * __formal=0x00400000)
 Line 1841 + 0x17	C++
 	mozilla.exe!WinMainCRTStartup()  Line 390 + 0x1b	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23

Comment 1

[Bastiaan Jacques]

Attachment: add error checking

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 189301 [details] [diff] [review]
add error checking

>Index: content/xbl/src/nsXBLPrototypeHandler.cpp
>   nsCOMPtr<nsIDOMScriptObjectFactory> factory =
>-    do_GetService(kDOMScriptObjectFactoryCID);
>-  NS_ENSURE_TRUE(factory, NS_ERROR_FAILURE);
>+    do_GetService(kDOMScriptObjectFactoryCID, &rv);
>+  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);

This change makes no sense.  Either return |rv|, or undo the change.

>-  nsCOMPtr<nsIJSEventListener> jsListener(do_QueryInterface(eventListener));
>+  nsCOMPtr<nsIJSEventListener> jsListener(do_QueryInterface(eventListener, &rv));
>+  NS_ENSURE_SUCCESS(rv, rv);

No need for that change.  Please undo it.

r+sr=bzbarsky with that.

Comment 3

[Bastiaan Jacques]

Attachment: address comments; patch for checkin

Comment 4

[Benjamin Smedberg]

Comment on attachment 189329 [details] [diff] [review]
address comments; patch for checkin

I would like bz to make sure this is what he meant before granting approval.

Comment 5

[Bastiaan Jacques]

Checked in by timeless (2005-07-17 11:50).