339916 - Coverity 464, leak after OOM in CERT_DistNamesFromNicknames

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

In CERT_DistNamesFromNicknames (file nss/lib/certhigh/certhigh.c )
after allocating "dnames", if the allocation of "names" gets NULL,
we leak dnames.

One question to investigate while working on this: 
Is this function dead code?  
If so, the solution is to remove it.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

This problem is much worse than Coverity indicated. 
All the things allocated in CERT_DistNamesFromNicknames are supposed to 
be part of the CERTDistNames object it returns, and are supposed to be
allocated from its one arenapool.  

When this object is destroyed, function CERT_FreeDistNames merely frees
the arena.   All the parts of the object are supposed to go away when 
that happens, but the parts not allocated from the arena will be leaked
at that time.  

So, the fix is not to free dnames at loser, but rather to ensure that 
dnames and names are both allocated from the arenapool, and that it is 
destroyed.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

Alexei, please review this patch very thoroughly, making sure that no
users of CERTDistNames objects depend on dnames or names being allocated
directly from the heap, rather than from an arenapool.

Comment 3

[Alexei Volkov]

Comment on attachment 224833 [details] [diff] [review]
patch v1

r=alexei

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Fix leaks in CERT_DistNamesFromNicknames (bug 339916) and in
CERT_GetCertNicknames (bug 339919). Coverity bugs. r=alexei.volkov

Checking in certhigh.c; new revision: 1.34.2.3; previous revision: 1.34.2.2
Checking in certhigh.c; new revision: 1.37;     previous revision: 1.36