Klocwork Null ptr dereferences in instance.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)

2.77 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
Klocwork ID 92337
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_DestroySessionHandle

Pointer 'fwSession' returned from call to function 'nssCKFWHash_Lookup' at 
line 671 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWSession_SetHandle' at line 675.

Klocwork ID 92338
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_ReassignObjectHandle

Pointer 'oldObject' returned from call to function 'nssCKFWHash_Lookup' at 
line 816 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWObject_SetHandle' at line 819.

Klocwork ID 92339
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_DestroyObjectHandle

Pointer 'fwObject' returned from call to function 'nssCKFWHash_Lookup' at 
line 857 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWObject_SetHandle' at line 861.
(Reporter)

Updated

11 years ago
Keywords: klocwork
(Reporter)

Updated

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

Comment 1

11 years ago
Created attachment 241206 [details] [diff] [review]
check pointer value returned by nssCKFWHash_Lookup
Assignee: nobody → alexei.volkov.bugs
Status: NEW → ASSIGNED
Attachment #241206 - Flags: review?(nelson)
(Reporter)

Comment 2

11 years ago
Comment on attachment 241206 [details] [diff] [review]
check pointer value returned by nssCKFWHash_Lookup

This change appears in two places:

>   fwSession = (NSSCKFWSession *)nssCKFWHash_Lookup(
>                 fwInstance->sessionHandleHash, (const void *)hSession);
>-
>+  if (!fwSession) {
>+      (void)nssCKFWMutex_Unlock(fwInstance->mutex);
>+      return;
>+  }
>+      
>   nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>   nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
> 
>   (void)nssCKFWMutex_Unlock(fwInstance->mutex);
> 
>   return;

In both places, I'd rather see

>   fwSession = (NSSCKFWSession *)nssCKFWHash_Lookup(
>                 fwInstance->sessionHandleHash, (const void *)hSession);
>-
>-  nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>-  nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
>+  if (fwSession) {
>+      nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>+      nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
>+  }
> 
>   (void)nssCKFWMutex_Unlock(fwInstance->mutex);
> 
>   return;
Attachment #241206 - Flags: review?(nelson) → review-
(Assignee)

Comment 3

11 years ago
Created attachment 241214 [details] [diff] [review]
fix as suggested
Attachment #241206 - Attachment is obsolete: true
Attachment #241214 - Flags: review?(nelson)
(Reporter)

Comment 4

11 years ago
Comment on attachment 241214 [details] [diff] [review]
fix as suggested

r=nelson for trunk
Attachment #241214 - Flags: review?(nelson) → review+
(Assignee)

Comment 5

11 years ago
/cvsroot/mozilla/security/nss/lib/ckfw/instance.c,v  <--  instance.c
new revision: 1.11; previous revision: 1.10
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.