Closed Bug 619268 Opened 9 years ago Closed 9 years ago

Memory leaks in CERT_ChangeCertTrust and CERT_SaveSMimeProfile

Categories

(NSS :: Libraries, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
3.12.9

People

(Reporter: ryan.sleevi, Assigned: ryan.sleevi)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch fixing the issue (obsolete) — Splinter Review
If either CERT_ChangeCertTrust or CERT_SaveSMimeProfile are called multiple times for the same certificate, and that certificate is not stored in a PKCS#11 module, then small leaks will happen when it makes it to the STAN DB.

The cause of this is that when assigning a trust setting or profile (nssCertificateStore_AddTrust or nssCertificateStore_AddSMIMEProfile), the existing NSSTrust/nssSMIMEProfile associated with the NSSCertificate is replaced, without dropping the reference that was previousl grabbed.

Because the arenas for both NSSTrust/nssSMIMEProfile are created on-demand when using CERT_ChangeCertTrust / CERT_SaveSMimeProfile, as opposed to being created in the same arena as the CERTCertificate/NSSCertificate, the underlying objects are lost and leaked.
Attachment #497728 - Flags: review?(wtc)
Thanks for the patch.  I guess we can't simply fix this with
+    if (entry->trust) {
+        nssTrust_Destroy(entry->trust);
+    }
     entry->trust = nssTrust_AddRef(trust);
because if 'trust' is the same as 'entry->trust', we must
not drop the reference count to 0 by accident.

I edited your patch for coding style and checked it in
on the NSS trunk (NSS 3.13).

Checking in pkistore.c;
/cvsroot/mozilla/security/nss/lib/pki/pkistore.c,v  <--  pkistore.c
new revision: 1.34; previous revision: 1.33
done
Attachment #497728 - Attachment is obsolete: true
Attachment #497728 - Flags: review?(wtc)
Bob, is it OK to include this memory leak fix in NSS 3.12.9?
If it's too late, I'll target NSS 3.12.10.
Assignee: nobody → ryan.sleevi
Status: NEW → ASSIGNED
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 3.12.9
Version: 3.12.8 → unspecified
if you get it in today, yes, otherwise let's put it 3.12.10.

bob
Patch checked in on the NSS_3_12_BRANCH (NSS 3.12.9).

Checking in pkistore.c;
/cvsroot/mozilla/security/nss/lib/pki/pkistore.c,v  <--  pkistore.c
new revision: 1.33.40.1; previous revision: 1.33
done
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.