The default bug view has changed. See this FAQ.

klocwork ptr dereference before NULL check in devutil.c

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
11 years ago
9 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.56 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
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.
(Reporter)

Updated

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

Comment 1

11 years ago
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.
Assignee: nobody → alexei.volkov.bugs
Status: NEW → ASSIGNED
Attachment #243815 - Flags: review?(nelson)
(Reporter)

Comment 2

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

Comment 3

11 years ago
/cvsroot/mozilla/security/nss/lib/dev/devutil.c,v  <--  devutil.c
new revision: 1.27; previous revision: 1.26
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

11 years ago
misspelling fix
/cvsroot/mozilla/security/nss/lib/dev/devutil.c,v  <--  devutil.c
new revision: 1.28; previous revision: 1.27

Updated

9 years ago
Depends on: 465270
You need to log in before you can comment on or make changes to this bug.