175200 - cert reference leaks in libsmime NSS_CMSSignerInfo functions

Status: RESOLVED INVALID

(NSS :: Libraries, defect, P1)

Description

[Nelson Bolyard (seldom reads bugmail)]

The functions

NSS_CMSSignerInfo_VerifyCertificate
NSS_CMSSignerInfo_Verify
NSS_CMSSignerInfo_GetSignerCommonName
NSS_CMSSignerInfo_GetSignerEmailAddress
NSS_SMIMESignerInfo_SaveSMIMEProfile

all call NSS_CMSSignerInfo_GetSigningCertificate which calls 
CERT_FindCertByIssuerAndSN, and returns the CERTCertificate pointer
returned from CERT_FindCertByIssuerAndSN.  

None of these functions appears to call CERT_DestroyCertificate to 
destroy the reference to the CERTCertificate.  Perhaps there is some 
mechanism that is destroying those references that I have overlooked?

Each time it is called, NSS_CMSSignerInfo_GetSigningCertificate stores a 
copy of the CERTCertificate pointer it gets back in the NSSCMSSignerInfo's
cert pointer member.  When the NSSCMSSignerInfo is destroyed by 
NSS_CMSSignerInfo_Destroy, that function calls CERT_DestroyCertificate once
to free the reference for the last pointer that was stored in its cert 
pointer member.  So, effectively, the reference returned during the last 
(or first, depending on your perspective) call to 
NSS_CMSSignerInfo_GetSigningCertificate is released by 
NSS_CMSSignerInfo_Destroy but all the other references are leaked.  

I think it is very likely that more than one of the functions named above
will be called on the same NSSCMSSignerInfo struct.  The reference obtained by 
first of these calls (or the last, depending on perspective) will not be leaked. 
 All the other references will be leaked.

I can think of a few ways to approach this problem.  

1. Elliminate the cert pointer in NSSCMSSignerInfo and make each function 
destroy any references it gets, or 

2. Have NSS_CMSSignerInfo_GetSigningCertificate keep a counter in the 
NSSCMSSignerInfo of the number of references to the signer cert that it gets, 
and then have NSS_CMSSignerInfo_Destroy call CERT_DestroyCertificate that
many times.

Comment 1

[Wan-Teh Chang]

Assigned the bug to Nelson.

If this cert reference leak is confirmed, we need to
check in the fix on NSS_3_6_BRANCH and should evaluate
whether NSS_3_5_BRANCH/MOZILLA_1_0_BRANCH needs the
fix as well.

Comment 2

[Kai Engert [:KaiE:]]

Thanks for finding this problem. It might be the cause for a bug 175161 I
reproduced yesterday. Adding dependency.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Marking invalid.  NSS_CMSSignerInfo_GetSigningCertificate only calls 
CERT_FindCertByIssuerAndSN if the NSSCMSSignerInfo's cert pointer is NULL.
So there should only be one reference obtained by 
NSS_CMSSignerInfo_GetSigningCertificate for each NSSCMSSignerInfo no matter 
how many times it is called, and that reference will be destroyed when
the NSSCMSSignerInfo is destroyed.  

However, I did find an apparent leak in NSS_SMIMESignerInfo_SaveSMIMEProfile.
I will add some more comments about that in bug 175161.