353780 - Klocwork NULL ptr dereferences in pkcs11.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

ID:       88800
Function: sftk_handleSMimeObject
Location: nss/lib/softoken/pkcs11.c : 983

Pointer 'email' returned from call to function 'sftk_getString' at 
line 969 may be NULL and may be dereferenced by passing argument 1 to 
function 'strlen' at line 983.

969	email = sftk_getString(object,CKA_NETSCAPE_EMAIL); 
978	if (rv != SECSuccess) { 
979	    PORT_Free(email); 
980	    return CKR_DEVICE_ERROR; 
981	} 
982	emailKey.data = (unsigned char *)email; 
983	emailKey.len = PORT_Strlen(email)+1; 
984	 
985	object->handle = sftk_mkHandle(slot, &emailKey, SFTK_TOKEN_TYPE_SMIME); 
986	PORT_Free(email); 

---

ID:       88992
Function: NSC_DeriveKey

Pointer 'slot' returned from call to function 'sftk_SlotFromSessionHandle' 
at line 4669 may be NULL and will be dereferenced by passing argument 1 to 
function sftk_NewObject at line 4702.  

4669	    SFTKSlot    *   slot	= sftk_SlotFromSessionHandle(hSession); 
4701	 
4702	    key = sftk_NewObject(slot); /* fill in the handle later */

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

ID:       88785
Function: sftk_handleCertObject
Location: nss/lib/softoken/pkcs11.c : 680

Pointer 'attribute' returned from call to function 'sftk_FindAttribute' at 
line 676 may be NULL and will be dereferenced at line 680.

676		attribute = sftk_FindAttribute(object,CKA_VALUE); 
677		PORT_Assert(attribute); 
678	 
679		derCert.type = 0; 
680		derCert.data = (unsigned char *)attribute->attrib.pValue; 

Note that klocwork tests non-DEBUG builds, so assertions don't appear in 
the preprocessor output.

----

ID:       88790
Function: sftk_handleTrustObject
Location: nss/lib/softoken/pkcs11.c : 819

Pointer 'issuer' returned from call to function 'sftk_FindAttribute' at 
line 817 may be NULL and will be dereferenced at line 819.

817	issuer = sftk_FindAttribute(object,CKA_ISSUER); 
818	PORT_Assert(issuer); 
819	issuerSN.derIssuer.data = (unsigned char *)issuer->attrib.pValue; 

----

ID:       88792
Function: sftk_handleTrustObject
Location: nss/lib/softoken/pkcs11.c : 824

Pointer 'serial' returned from call to function 'sftk_FindAttribute' at 
line 822 may be NULL and will be dereferenced at line 824.

822	serial = sftk_FindAttribute(object,CKA_SERIAL_NUMBER); 
823	PORT_Assert(serial); 
824	issuerSN.serialNumber.data = (unsigned char *)serial->attrib.pValue; 

----

ID:       88797
Function: sftk_handleSMimeObject
Location: nss/lib/softoken/pkcs11.c : 946

Pointer 'subject' returned from call to function 'sftk_FindAttribute' at line 944 may be NULL and will be dereferenced at line 946.

944	subject = sftk_FindAttribute(object,CKA_SUBJECT); 
945	PORT_Assert(subject); 
946	derSubj.data = (unsigned char *)subject->attrib.pValue; 

----

ID:       88805
Function: sftk_handleCrlObject
Location: nss/lib/softoken/pkcs11.c : 1037

Pointer 'crl' returned from call to function 'sftk_FindAttribute' at 
line 1035 may be NULL and will be dereferenced at line 1037.

1035	crl = sftk_FindAttribute(object,CKA_VALUE); 
1036	PORT_Assert(crl); 
1037	derCrl.data = (unsigned char *)crl->attrib.pValue; 

Comment 2

[Alexei Volkov]

Attachment: fixes

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 243709 [details] [diff] [review]
fixes

I found one problem in the patch to sftk_handleSMimeObject

>@@ -925,31 +931,40 @@
> 
>     if (sftk_isTrue(object,CKA_TOKEN)) {
> 	SFTKSlot *slot = session->slot;
> 	SECItem derSubj,rawProfile,rawTime,emailKey;
> 	SECItem *pRawProfile = NULL;
> 	SECItem *pRawTime = NULL;
> 	char *email = NULL;
>     	SFTKAttribute *subject,*profile,*time;

The above 3 pointers are not initialized to NULL, and so may contain
random values when tested for NULL in the new code below.

>+loser:
>+	sftk_freeCertDB(certHandle);
>+	if (subject) sftk_FreeAttribute(subject);
>+	if (profile) sftk_FreeAttribute(profile);
>+	if (time)    sftk_FreeAttribute(time);
>+	if (email)   PORT_Free(email);
>+
>+	return ck_rv
>     }

Comment 4

[Alexei Volkov]

Attachment: init subject, time and profile pointers with NULL

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 244497 [details] [diff] [review]
init subject, time and profile pointers with NULL

r=nelson

Comment 6

[Alexei Volkov]

/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.138; previous revision: 1.137
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v  <--  pkcs11c.c
new revision: 1.92; previous revision: 1.91