Null pointer dereference in |PR_OpenDir| (pr/src/pthreads/ptio.c)

RESOLVED FIXED in 4.7

Status

--
minor
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: kherron+mozilla, Assigned: sciguyryan)

Tracking

({coverity})

other
coverity

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
This was found through a coverity scan of the firefox source code. Please refer to the sample URL.

|PR_OpenDir| calls |PR_NEWZAP| to allocate a PRDir structure. |PR_NEWZAP| wraps |PR_Calloc|, which may call |calloc|, which may return NULL. |PR_OpenDir| then dereferences the pointer returned from |PR_NEWZAP| without testing it for NULL.
(Assignee)

Comment 1

12 years ago
Created attachment 242199 [details] [diff] [review]
Patch v1

Only preform |dir->md.d = osdir| if |dir| is not null. Seeing as as |PR_OpenDir| can already return null (and thus any use should check for a null return) it should be OK to just skip and return a null for |dir|.
Attachment #242199 - Flags: superreview?(wtchang)
Attachment #242199 - Flags: review?(darin)

Comment 2

12 years ago
Comment on attachment 242199 [details] [diff] [review]
Patch v1

Thanks for the patch.  Please add a
closedir(osdir) call if dir is NULL.
Attachment #242199 - Flags: superreview?(wtchang)
Attachment #242199 - Flags: superreview-
Attachment #242199 - Flags: review?(darin)
(Assignee)

Comment 3

12 years ago
Created attachment 242203 [details] [diff] [review]
Patch v2

Updated patch to Wan-Teh Chang's comment. Added |closedir(osdir)| if |dir| is null.
Assignee: wtchang → sciguyryan+bugzilla
Attachment #242199 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #242203 - Flags: superreview?(wtchang)
Attachment #242203 - Flags: review?(wtchang)

Updated

12 years ago
Attachment #242203 - Flags: superreview?(wtchang)
Attachment #242203 - Flags: superreview+
Attachment #242203 - Flags: review?(wtchang)
Attachment #242203 - Flags: review+
(Assignee)

Updated

12 years ago
Whiteboard: [checkin needed]
I can't check this patch in, because NSPR is a restricted partition. Wtc, could you check the patch in on the trunk and any appropriate branches?

Comment 5

12 years ago
Created attachment 242521 [details] [diff] [review]
Patch as checked in

I checked in the patch (with a (void) cast to indicate
that we know we are ignoring the return value of closedir)
on the NSPR trunk (NSPR 4.7) and the NSPRPUB_PRE_4_2_CLIENT_BRANCH
(Mozilla 1.9 alpha).

Checking in ptio.c;
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v  <--  ptio.c
new revision: 3.105; previous revision: 3.104
done

Checking in ptio.c;
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v  <--  ptio.c
new revision: 3.71.2.26; previous revision: 3.71.2.25
done
Attachment #242203 - Attachment is obsolete: true

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → 4.7
You need to log in before you can comment on or make changes to this bug.