The default bug view has changed. See this FAQ.

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.