Closed Bug 469619 Opened 12 years ago Closed 11 years ago

Coverity errors reported in libSMIME

Categories

(NSS :: Libraries, defect, P2)

3.12
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.12.7

People

(Reporter: nelson, Assigned: shailen.n.jain)

Details

(Keywords: coverity, memory-leak, regression)

Attachments

(1 file, 2 obsolete files)

Coverity reports some new bugs in libSMIME.  Appear to be regressions.
Some of the code changes appear to have been part of the work done for 
bug 401928 - Support generalized PKCS#5 v2 PBEs
I will add info about those bugs as comments to this bug

CID 1352 - memory leak

In file nss/lib/pkcs7/p7local.c, function sec_PKCS7CreateDecryptObject
Coverity reports:

127 	cryptoMechType = PK11_GetPBECryptoMechanism(algid, &param, pwitem);
128 	if (cryptoMechType == CKM_INVALID_MECHANISM) {
129 	    PORT_Free(result);
130  	    return NULL;
131  	}

Event leaked_storage: Returned without freeing storage "param"
CID 1351 - memory leak

In file nss/lib/pkcs7/p7local.c, function sec_PKCS7CreateEncryptObject
(same problem as CID 1352 above, except different function.)

202  	cryptoMechType = PK11_GetPBECryptoMechanism(algid, &param, pwitem);
203  	if (cryptoMechType == CKM_INVALID_MECHANISM) {
204  	    PORT_Free(result);
205  	    return NULL;
206  	}

Event leaked_storage: Returned without freeing storage "param"
Also see events: [alloc_arg]
CID 1353 - memory leak

File nss/lib/smime/cmscipher.c Function NSS_CMSCipherContext_StartDecrypt

102  	cryptoMechType = PK11_GetPBECryptoMechanism(algid, &param, pwitem);
103  	if (cryptoMechType == CKM_INVALID_MECHANISM) {
104  	    return NULL;
105  	}

Event leaked_storage: Returned without freeing storage "param"
CID 1354 - memory leak

file nss/lib/smime/cmscipher.c function NSS_CMSCipherContext_StartEncrypt

172  	cryptoMechType = PK11_GetPBECryptoMechanism(algid, &param, pwitem);
173  	if (cryptoMechType == CKM_INVALID_MECHANISM) {
174  	    return NULL;
175  	}

Event leaked_storage: Returned without freeing storage "param"

Since all 4 of these CIDs are due to PK11_GetPBECryptoMechanism, 
I'm assigning this bug to Bob
Assignee: nobody → rrelyea
Priority: -- → P2
Target Milestone: 3.12.3 → 3.12.5
Attached patch Patch V 1 (obsolete) — Splinter Review
Nelson:

 Can you please review the patch and let me know if anything is missing.

Regards,
Shailendra
Attachment #430914 - Flags: review?(nelson)
Comment on attachment 430914 [details] [diff] [review]
Patch V 1

param must be initialized to NULL in all these cases, or else it can be uninitialized when it is passed to SECITEM)FreeItem.
Attachment #430914 - Flags: review?(nelson) → review-
shailen.n.jain@gmail.com: i've given you canconfirm+editbugs. please use these bits wisely. mostly for now, it'll mean that when you attach a patch, bugzilla will let you change the owner to yourself and change the state to 'assigned'.
Assignee: rrelyea → shailen.n.jain
Status: NEW → ASSIGNED
Attached patch Patch Version 2 (obsolete) — Splinter Review
Nelson :

  I have initialized the param variable.

  Please review and let me know if any changes required.

Thanks,
Shailendra
Attachment #430914 - Attachment is obsolete: true
Attachment #431050 - Flags: review?(nelson)
Attached patch Patch Version 3Splinter Review
Nelson :

 For some reason the file path for cmscipher.c was selected as lib/pk11wrap/ instead of ./lib/smime in the patch that I posted.

  So I am attaching the newer version of the patch.

Thanks and Regards,
Shailendra
Attachment #431050 - Attachment is obsolete: true
Attachment #431053 - Flags: review?(nelson)
Attachment #431050 - Flags: review?(nelson)
Comment on attachment 431053 [details] [diff] [review]
Patch Version 3

r=nelson
Attachment #431053 - Flags: review?(nelson) → review+
Checking in pkcs7/p7local.c;   new revision: 1.14; previous revision: 1.13
Checking in smime/cmscipher.c; new revision: 1.13; previous revision: 1.12
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: 3.12.5 → 3.12.7
You need to log in before you can comment on or make changes to this bug.