353739 - Klocwork Null ptr dereferences in instance.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 1

[Alexei Volkov]

Attachment: check pointer value returned by nssCKFWHash_Lookup

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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;

Comment 3

[Alexei Volkov]

Attachment: fix as suggested

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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

r=nelson for trunk

Comment 5

[Alexei Volkov]

/cvsroot/mozilla/security/nss/lib/ckfw/instance.c,v  <--  instance.c
new revision: 1.11; previous revision: 1.10