Last Comment Bug 353780 - Klocwork NULL ptr dereferences in pkcs11.c
: Klocwork NULL ptr dereferences in pkcs11.c
Status: RESOLVED FIXED
: klocwork
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P2 normal (vote)
: 3.12
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-22 01:31 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2007-01-04 16:22 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fixes (13.88 KB, patch)
2006-10-26 14:51 PDT, Alexei Volkov
nelson: review-
Details | Diff | Review
init subject, time and profile pointers with NULL (14.06 KB, patch)
2006-11-02 14:32 PST, Alexei Volkov
nelson: review+
Details | Diff | Review

Description Nelson Bolyard (seldom reads bugmail) 2006-09-22 01:31:50 PDT
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) 2006-09-23 21:27:16 PDT
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 2006-10-26 14:51:23 PDT
Created attachment 243709 [details] [diff] [review]
fixes
Comment 3 Nelson Bolyard (seldom reads bugmail) 2006-10-30 14:58:59 PST
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 2006-11-02 14:32:08 PST
Created attachment 244497 [details] [diff] [review]
init subject, time and profile pointers with NULL
Comment 5 Nelson Bolyard (seldom reads bugmail) 2006-11-02 15:14:20 PST
Comment on attachment 244497 [details] [diff] [review]
init subject, time and profile pointers with NULL

r=nelson
Comment 6 Alexei Volkov 2007-01-04 16:22:04 PST
/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

Note You need to log in before you can comment on or make changes to this bug.