Closed Bug 603302 Opened 9 years ago Closed 9 years ago

checking for observerID.IsEmpty() is bad in nsXMLEventsListener::InitXMLEventsListener

Categories

(Core :: DOM: Events, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [sg:nse])

Attachments

(1 file)

54 PRBool nsXMLEventsListener::InitXMLEventsListener(nsIDocument * aDocument,
55                                                   nsXMLEventsManager * aManager,
56                                                   nsIContent * aContent)
57 {

74   nsAutoString observerID;
106   PRBool hasObserver = 
107     aContent->GetAttr(nameSpaceID, nsGkAtoms::observer, observerID);

121   nsIContent *observer;
122   if (!hasObserver) {
123     if (!hasHandlerURI) //Parent should be the observer
124       observer = aContent->GetParent();
125     else //We have the handler, so this is the observer
126       observer = aContent;
127   }
128   else if (!observerID.IsEmpty()) {
129     observer = aDocument->GetElementById(observerID);
130   }
131   nsCOMPtr<nsIDOMEventTarget> eventObserver(do_QueryInterface(observer));

The code as written can only initialize observer if !hasObserver or !observerID.IsEmpty(). I think it's safe to assert that observerID.IsEmpty() will be false if hasObserver. Worst case, GetElementById() returns null. Currently the code pretends to send an uninitialized pointer to do_QueryInterface() which disturbs coverity.
Group: core-security
Timeless, want to write the patch?
And please, no useless assertions - removing the .IsEmpty check should be enough.
This is a regression from bug 572609
Attachment #482306 - Flags: review?(jst)
Whiteboard: [sg:nse]
Attachment #482306 - Flags: review?(jst)
Attachment #482306 - Flags: review+
Attachment #482306 - Flags: approval2.0+
http://hg.mozilla.org/mozilla-central/rev/4d33e20a07d1
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Group: core-security
Assignee: nobody → bugs
You need to log in before you can comment on or make changes to this bug.