135058 - Leaked slot references in lib/pk11wrap/pk11cert.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

In PK11_FindCertFromNickname(), there are two places
where we return NULL without calling PK11_FreeSlot().

1.
    if (!PK11_IsPresent(slot)) {
        return NULL;
    }

2.
    /* find matching certs on the token */
    certList = nssList_Create(NULL, PR_FALSE);
    if (!certList) return NULL;

It seems that the slot reference would be leaked.

Comment 1

[Wan-Teh Chang]

PK11_FindCertsFromNickname() in the same file has the same
problem.  It does not free the slot reference here:

1554             if (!nameList) return NULL;

Comment 2

[Wan-Teh Chang]

PK11_SaveSMimeProfile() calls PK11_GetInternalKeySlot() but
does not free the slot reference.

There is a comment that says it should be done :-)

3988     if (slot == NULL) {
3989         slot = PK11_GetInternalKeySlot();
3990         /* we need to free the key slot in the end!!! */
3991     }

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch

I decided to use "goto loser" in PK11_FindCertFromNickname
and PK11_FindCertsFromNickname because they also need to
free 'nickCopy' before returning.

Comment 4

[Wan-Teh Chang]

Changed the QA contact to Bishakha.

Comment 5

[Wan-Teh Chang]

Attachment: Patch to free nickCopy on error returns in PK11_FindCertsFromNickname

I verified that in the current revision (1.80) of pk11cert.c
all the slot reference leaks I reported have been fixed.

The only thing left to do is to free the string 'nickCopy'
on error returns in PK11_FindCertsFromNickname, which is
what this patch does.

Please review.	Thanks.

Comment 6

[Wan-Teh Chang]

Patch checked into the tip of NSS.

Comment 7

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) for checkin to the 1.0 branch. Pls check this in
asap. thanks!