potential null-pointer dereference in nsDocument::ElementFromPointHelper

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
DOM
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

Trunk
mozilla1.9.2a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 366537 [details] [diff] [review]
fix while() condition to avoid possible null-deref

This function includes the fragment:

  while (ptContent &&
         !ptContent->IsNodeOfType(nsINode::eELEMENT) ||
         ptContent->IsInAnonymousSubtree()) {
    // XXXldb: Faster to jump to GetBindingParent if non-null?
    ptContent = ptContent->GetParent();
  }

(see http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#2674).

This code is incorrect because && has higher precedence than ||, and therefore if ptContent ever becomes NULL during this loop, it will still attempt to call IsInAnonymousSubtree(). It seems clear that the intention was to check for non-NULL ptContent before calling either of the Is... methods.

I don't have an actual test-case that encounters this situation; perhaps it cannot arise in actual practice. However, the fact that the code tests for non-NULL ptContent at all (both here, and before calling CallQueryInterface in the next couple of lines of code) implies that it is considered possible. Either these tests are completely redundant, or the while() code is unsafe.

The attached patch fixes the unsafe test by simply adding parens around the || alternatives, providing the semantics that were presumably intended all along.
Attachment #366537 - Flags: review?
Comment on attachment 366537 [details] [diff] [review]
fix while() condition to avoid possible null-deref

I guess you were going to ask review from someone.
Attachment #366537 - Flags: superreview+
Attachment #366537 - Flags: review?
Attachment #366537 - Flags: review+
(Assignee)

Comment 2

9 years ago
Uhhhh.... yeah! Thanks.
Assignee: nobody → jfkthame
Keywords: checkin-needed
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/cf256da7436b
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.