175161 - NSS shutdown fails after having sent a signed S/Mime message

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Kai Engert [:KaiE:]]

Tested using the patch from bug 157937.

Prepare a profile for being able to send signed S/Mime messages.
Restart the application.
Use File/New Messages
Select to sign the message.
Send the message to yourself.
Quit the application.

Actual behaviour: The checking code reports that NSS shutdown failed.

Expected behaviour: Shutdown should work correctly.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

I found an apparent leak in NSS_SMIMESignerInfo_SaveSMIMEProfile.
That function has a cert variable whose value comes from one of two functions
NSS_SMIMEUtil_GetCertFromEncryptionKeyPreference and 
NSS_CMSSignerInfo_GetSigningCertificate.  

NSS_SMIMEUtil_GetCertFromEncryptionKeyPreference returns a new reference to the 
cert, which must be destroyed or it will leak.

NSS_CMSSignerInfo_GetSigningCertificate returns a cert pointer, but the 
reference is held inside the CMSSignerInfo struct, and will be destroyed 
when that struct is destroyed.

This function never calls CERT_DestroyCertificate.  If the cert pointer comes 
from NSS_SMIMEUtil_GetCertFromEncryptionKeyPreference then this function
should destroy the cert reference.  

Kai, I'd like you to do a test for me.  Please set a breakpoint in 
NSS_SMIMESignerInfo_SaveSMIMEProfile at the call to 
NSS_SMIMEUtil_GetCertFromEncryptionKeyPreference and see which, if any,
PSM SMIME operations hit that breakpoint, and whether NSS_Shutdown fails after
doing so.  

Comment 2

[Kai Engert [:KaiE:]]

Nelson, thanks a lot for finding this leak.
I have filed NSS bug 176799 for fixing the leak in NSS.
However, it does not fix this bug 175161, but it fixes bug 175159.

Comment 3

[Kai Engert [:KaiE:]]

This should work after bug 177260 has been fixed.

Comment 4

[Kai Engert [:KaiE:]]

Marking fixed, because both bug 177260 and bug 189025 have been landed.
Fix can be verified by using a special test build based on the latest trunk,
that includes the profile switching patch from bug 97622 (I'll provide such a
build and add a comment to bug 97622).

Comment 5

[John Unruh]

Verified fixed.