Klocwork NULL ptr dereferences in pkcs11.c

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Alexei Volkov)

Tracking

({klocwork})

trunk
3.12
klocwork

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

14.06 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
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 */
(Reporter)

Updated

11 years ago
Summary: Klocwork NULL ptr dereferences in pk11c.c → Klocwork NULL ptr dereferences in pkcs11c.c
(Reporter)

Updated

11 years ago
Summary: Klocwork NULL ptr dereferences in pkcs11c.c → Klocwork NULL ptr dereferences in pkcs11.c
(Reporter)

Comment 1

11 years ago
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; 
(Reporter)

Updated

11 years ago
Priority: -- → P2
Target Milestone: --- → 3.12
(Assignee)

Comment 2

11 years ago
Created attachment 243709 [details] [diff] [review]
fixes
Assignee: nobody → alexei.volkov.bugs
Status: NEW → ASSIGNED
Attachment #243709 - Flags: review?(nelson)
(Reporter)

Comment 3

11 years ago
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
>     }
Attachment #243709 - Flags: review?(nelson) → review-
(Assignee)

Comment 4

11 years ago
Created attachment 244497 [details] [diff] [review]
init subject, time and profile pointers with NULL
Attachment #243709 - Attachment is obsolete: true
Attachment #244497 - Flags: review?(nelson)
(Reporter)

Comment 5

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

r=nelson
Attachment #244497 - Flags: review?(nelson) → review+
(Assignee)

Comment 6

11 years ago
/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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.