Open Bug 298649 Opened 19 years ago Updated 2 years ago

SECITEM_ReallocItem leaks mem, doesn't set item length

Categories

(NSS :: Libraries, defect, P3)

3.10

Tracking

(Not tracked)

People

(Reporter: nelson, Unassigned)

Details

(Keywords: memory-leak)

Lots of bugs in SECITEM_ReallocItem.

a) SECITEM_ReallocItem does NOT set the "len" field in the reallocated 
item to the new length, but leaves it with the old value, UNLESS the 
item length is initially zero (and hence SECITEM_ReallocItem is really 
being used to allocate a new buffer), then it sets the item's length.
SECITEM_ReallocItem really should set item->len upon success.

b) If the realloc fails, the old buffer is lost/leaked.  In that case,
SECITEM_ReallocItem return SECFailure AND sets item->data to NULL. :(
SECITEM_ReallocItem should preserve the old buffer address and length
unless and until the realloc succeeds.  

Although SECITEM_ReallocItem returns a SECStatus, programs may intead
be relying on its behavior of setting item->data to NULL to indicate
failure.  If this behavior is "fixed" to not change item->data upon
failure, programs may incorrectly interpret a Non-NULL item->data as an
indication of success.  :(
I found that bltest is the only users of SECITEM_Realloc
in the whole Mozilla CVS repository.

Problem a (not setting the len field) is serious.  The
author of bltest ran into this problem and implemented a
workaround.  So fixing Problem a won't break bltest.

Problem b is less serious because when we run out of memory
(which is rare) most apps will crash sooner or later anyway.

But the combination of a and b makes it hard to fix this
bug without breaking existing users of SECITEM_ReallocItem.
I'm not sure what's the right thing to do.
Assignee: wtchang → nobody
QA Contact: jason.m.reid → libraries
Priority: -- → P3
Keywords: mlk
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.