Last Comment Bug 353909 - klocwork ptr dereference before NULL check in devutil.c
: klocwork ptr dereference before NULL check in devutil.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: 465270
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-22 18:23 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2008-11-16 22:22 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
preventive measure (1.56 KB, patch)
2006-10-27 11:26 PDT, Alexei Volkov
nelson: review+
Details | Diff | Review

Description Nelson Bolyard (seldom reads bugmail) 2006-09-22 18:23:38 PDT
ID:       76974
Function: create_object
Location: nss/lib/dev/devutil.c : 716

Suspicious dereference of pointer 'slot' by passing argument 6 to function 'nssCKObject_GetAttributes' at line 716 before NULL check at line 727

688	    NSSSlot *slot = NULL; 
692	    slot = nssToken_GetSlot(object->token); 
716	    *status = nssCKObject_GetAttributes(object->handle, 
717	                                        rvCachedObject->attributes, 
718	                                        numTypes, 
719	                                        arena, 
720	                                        session, 
721	                                        slot); 
727	    if (slot) { 
728		nssSlot_Destroy(slot); 
729	    } 
730	    return rvCachedObject; 
731	loser: 

This is not the usual case of a null check in loser.
Comment 1 Alexei Volkov 2006-10-27 11:26:08 PDT
Created attachment 243815 [details] [diff] [review]
preventive measure

I don't see how slot can be optional when we call nssCKObject_GetAttributes. Looks like klocwork makes assumption that slot can be NULL based if statement, that follows the function call.

Patch adds preventive slot check for NULL and also removes redundant if statement.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-10-30 15:08:45 PST
Comment on attachment 243815 [details] [diff] [review]
preventive measure


>     slot = nssToken_GetSlot(object->token);
>+    if (!slot) {
>+        nss_SetError(NSS_ERROR_INVALUD_POINTER);

I'm pretty sure that patch won't even compile.  
You should at least compile patches before requesting review. :)

But when that spelling error is checked, then r+ for this patch.
Comment 3 Alexei Volkov 2006-10-30 16:05:16 PST
/cvsroot/mozilla/security/nss/lib/dev/devutil.c,v  <--  devutil.c
new revision: 1.27; previous revision: 1.26
Comment 4 Alexei Volkov 2006-10-30 16:23:05 PST
misspelling fix
/cvsroot/mozilla/security/nss/lib/dev/devutil.c,v  <--  devutil.c
new revision: 1.28; previous revision: 1.27

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