71283 - nsEventListenerManager::AddEventListener crashes if passed null listener

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Alex Fritze]

... because of a missing null-pointer check. Same for RemoveEventListener.
Trival fix:

Index: nsEventListenerManager.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/events/src/nsEventListenerManager.cpp,v
retrieving revision 1.87
diff -c -7 -r1.87 nsEventListenerManager.cpp
*** nsEventListenerManager.cpp	2001/01/23 03:10:25	1.87
--- nsEventListenerManager.cpp	2001/03/08 10:35:19
***************
*** 237,250 ****
--- 237,252 ----
  nsresult nsEventListenerManager::AddEventListener(nsIDOMEventListener 
*aListener, 
                                                    const nsIID& aIID, 
                                                    PRInt32 aFlags,
                                                    PRInt32 aSubType)
  {
    nsVoidArray** listeners = GetListenersByIID(aIID);
  
+   if (nsnull == listeners) return NS_ERROR_FAILURE;
+ 
    if (nsnull == *listeners) {
      *listeners = new nsVoidArray();
    }
  
    if (nsnull == *listeners) {
      return NS_ERROR_OUT_OF_MEMORY;
    }
***************
*** 314,327 ****
--- 316,331 ----
  
  nsresult nsEventListenerManager::RemoveEventListener(nsIDOMEventListener 
*aListener, 
                                                       const nsIID& aIID, 
                                                       PRInt32 aFlags,
                                                       PRInt32 aSubType)
  {
    nsVoidArray** listeners = GetListenersByIID(aIID);
+ 
+   if (nsnull == listeners) return NS_ERROR_FAILURE;
  
    if (nsnull == *listeners) {
      return NS_OK;
    }
  
    nsListenerStruct* ls;
    nsresult rv;

Comment 1

[Kathleen :Brade]

confirm bug to get on the radar

Comment 2

[Håkan Waara]

Regarding the fix, you should return |NS_ERROR_NULL_POINTER| for nullpointers.

Comment 3

[Alex Fritze]

Surely you should return NS_ERROR_NULL_POINTER only if the function was 
actually passed a null pointer of sorts as an argument.

NS_ERROR_INVALID_ARG is probably the right choice here?

Comment 4

[joki (gone)]

Adding to 0.9 list.  Will check in fix when tree opens.

Comment 5

[Keyser Sose]

Has this been checked in?

Comment 6

[Keyser Sose]

*** Bug 74732 has been marked as a duplicate of this bug. ***

Comment 7

[joki (gone)]

Sorry, didn't check in patch alone as it got wrapped into a larger patch/rewrite 
of the surrounding code.

Comment 8

[joki (gone)]

Create/Init/Dispatch bugs all fixed.  Patches are attached to bug 25889.

Comment 9

[Vladimir Ermakov]

Still crashing at http://www.protocol7.com/scroll/nsbug.html, Testcase from a 
duplicate bug #74732. (build 2001-04-19-04-trunk windows 98)

Comment 10

[joki (gone)]

Okay, same function different crash.  Close enough to be a dupe.  I'll see if I 
can get a fix allowed in.

Comment 11

[joki (gone)]

Attachment: proposed patch (just a one line null check)

Comment 12

[Johnny Stenback (:jst)]

[s]r=jst

Comment 13

[Asa Dotzler [:asa]]

Are you still shooting for 0.9 on this? If so please email [e-mail]
with a status on you progress. If not please retarget against a later Milestone.
Thanks.

Comment 14

[joki (gone)]

Checked in with approval from choffmann from friday.

Comment 15

[Madhur Bhatia]

QA contact updated

Comment 16

[luis rincon]

verified on build 2001-07-30-10-trunk