Null pointer dereference or redundant check in mailnews/mime/src/mimei.cpp.

RESOLVED FIXED in Thunderbird 3.1a1

Status

MailNews Core
MIME
--
minor
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Adam Buchbinder, Assigned: Adam Buchbinder)

Tracking

({fixed-seamonkey2.0.4})

Trunk
Thunderbird 3.1a1
fixed-seamonkey2.0.4
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

(thunderbird3.0 .2-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091109 Ubuntu/9.10 (karmic) Firefox/3.5.5
Build Identifier: thunderbird-3.0rc1

cppcheck, a static code analysis tool, reported:

[comm-1.9.1/mailnews/mime/src/mimei.cpp:1502]: (error) Possible null pointer dereference: options - otherwise it is redundant to check if options is null at line 1505

Reproducible: Always




In mime_parse_url_options(), the 'options' argument is a pointer. It's used by calling "options->headers" before being checked with "if (!options)". If it's expected that options could be null, the assignment to default_headers should be moved below the check; if it's can never be null, the check is redundant and can be removed.

Either way, the test as it stands does nothing; if 'options' is null, it will simply lead to a null pointer reference and subsequent crash.

I'm marking this as minor, because I haven't been able to generate an actual crash. Please advise if I should mark this sort of issue with a different severity.
Component: General → MIME
Product: Thunderbird → MailNews Core
QA Contact: general → mime
(Assignee)

Updated

8 years ago
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 1

8 years ago
Created attachment 419718 [details] [diff] [review]
Patch to move the assignment below the check.

I've used the simplest change; the assignment now occurs after the check, so it's safe against 'options' being null.
Attachment #419718 - Flags: superreview?(bugzilla)
Attachment #419718 - Flags: review?(bienvenu)
(Assignee)

Updated

8 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

8 years ago
Comment on attachment 419718 [details] [diff] [review]
Patch to move the assignment below the check.

thx for the patch, Adam - mozilla style would be to move the declaration to be with the assignment, instead of splitting them up. r=me with that change.
Attachment #419718 - Flags: review?(bienvenu) → review-
(Assignee)

Comment 3

8 years ago
Created attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

Edited the patch so that the assignment is together with the declaration, per Mozilla style.
Assignee: nobody → adam.buchbinder
Attachment #419718 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #419902 - Flags: superreview?(bugzilla)
Attachment #419902 - Flags: review?(bienvenu)
Attachment #419718 - Flags: superreview?(bugzilla)

Updated

8 years ago
Attachment #419902 - Flags: review?(bienvenu) → review+

Comment 4

8 years ago
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

thx, Adam.
Attachment #419902 - Flags: superreview?(bugzilla) → superreview+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]


http://hg.mozilla.org/comm-central/rev/3ed8bb7b219b
Attachment #419902 - Attachment description: Patch to move the assignment below the check. → Move the assignment below the check [Checkin: Comment 5]
Attachment #419902 - Flags: approval-thunderbird3.0.1?
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1a1
Version: unspecified → Trunk
Comment on attachment 419902 [details] [diff] [review]
Move the assignment below the check
[Checkin: Comment 5 & 7]

We're not seeing any crashers in this area currently. We've already got a significant amount for 3.0.1 so I don't want to add this as well even thought it may be small.
Attachment #419902 - Flags: approval-thunderbird3.0.2?
Attachment #419902 - Flags: approval-thunderbird3.0.1?
Attachment #419902 - Flags: approval-thunderbird3.0.1-
Attachment #419902 - Flags: approval-thunderbird3.0.2? → approval-thunderbird3.0.2+
Checked in: http://hg.mozilla.org/releases/comm-1.9.1/rev/b3812420fcab
status-thunderbird3.0: --- → .2-fixed
Attachment #419902 - Attachment description: Move the assignment below the check [Checkin: Comment 5] → Move the assignment below the check [Checkin: Comment 5 & 7]

Updated

7 years ago
Keywords: fixed-seamonkey2.0.4
You need to log in before you can comment on or make changes to this bug.