344787 - NULL pointer can be dereferenced in nsHTMLFormElement.cpp

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[junkmailnotread]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060606 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060606 Firefox/1.5.0.4

While reading the current revision (1.203) of nsHTMLFormElement.cpp:

content/html/content/src/nsHTMLFormElement.cpp

I noticed an instance where a NULL pointer can be dereferenced. Here's the code snippet with line numbers added:

   929    nsIDocument* doc = GetCurrentDoc();
   930    if (!doc) {
   931      // Nothing to do
   932    }
   933  
   934    nsCOMPtr<nsISupports> container = doc->GetContainer();

If GetCurrentDoc() returns a NULL pointer in line 929, and thus the test in line 930 evaluates to TRUE, then a NULL pointer will be dereferenced in line 934. This will obviously crash the process.

This bug is theoretical. It's possible that GetCurrentDoc() never returns a NULL pointer. However the test in line 930 suggests that it can.

Reproducible: Didn't try

Comment 1

[timeless]

if the function wasn't supposed to return null, it'd be called CurrentDoc() instead.

for reference, the code you're reading is from Core not Firefox.
firefox code is mostly mozilla/browser
toolkit code is mostly mozilla/toolkit
most other code is core.

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

This code was changed by bug 100533.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Mats, thanks for fixing this!  Please get it in on the 1.8 branches too?

Comment 4

[Mats Palmgren (inactive)]

(In reply to comment #3)
> Please get it in on the 1.8 branches too?

Will ask for approval after it has baked a couple of days.

Checked in to trunk at 2006-07-22 04:07 PDT.

-> FIXED

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 229363 [details] [diff] [review]
Patch rev. 1

Simple null-check fix, zero risk.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 229363 [details] [diff] [review]
Patch rev. 1

a=drivers. Please land this on the MOZILLA_1_8_BRANCH.

Comment 7

[Mats Palmgren (inactive)]

It's not needed on the 1.8 branch yet, since bug 100533 hasn't landed there yet.

Comment 8

[Benjamin Smedberg]

No need to block 1.8.1 since 100533 hasn't landed there.

Comment 9

[Mike Schroepfer]

Comment on attachment 229363 [details] [diff] [review]
Patch rev. 1

Clearing a flag since bug 100533 never landed on 1.8.1 branch and thus this shouldn't