crash [@ pk11_DoKeys] "arg" Pointer dereferenced before NULL check

RESOLVED FIXED in 3.11.2

Status

P3
trivial
RESOLVED FIXED
13 years ago
7 years ago

People

(Reporter: timeless, Assigned: alvolkov.bgs)

Tracking

({coverity, crash})

3.11
3.11.2
coverity, crash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [CID 312], crash signature, URL)

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
 
(Reporter)

Comment 1

13 years ago
Created attachment 221197 [details] [diff] [review]
look before leaping
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #221197 - Flags: review?(nelson)
I'm surprised this is listed as "crash" and not as "useless NULL check" or 
even "dead code".  

An inspection of the code shows that there is only one path into pk11_DoKeys,
PK11_TraversePrivateKeysInSlot calls PK11_TraverseSlot passing pk11_DoKeys
and the non-null stack address of a data structure to it.  PK11_TraverseSlot
then calls pk11_DoKeys with that structure address, which cannot be NULL.

So, I'm downgrading the severity to "trivial".  
Severity: critical → trivial
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
Comment on attachment 221197 [details] [diff] [review]
look before leaping

r=nelson

>     pk11KeyCallback *keycb = (pk11KeyCallback *) arg;
>+    if (!arg) {

Should set an error code here.  But caller will ignore it.

>+        return SECFailure;
>+    }
Attachment #221197 - Flags: review?(nelson) → review+
Target Milestone: --- → 3.11.2
(Reporter)

Updated

13 years ago
Assignee: timeless → alexei.volkov.bugs
Status: ASSIGNED → NEW
(Assignee)

Comment 4

13 years ago
trunk
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11akey.c,v  <--  pk11akey.c
new revision: 1.14; previous revision: 1.13

3.11 branch
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11akey.c,v  <--  pk11akey.c
new revision: 1.9.2.4; previous revision: 1.9.2.3
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 5

13 years ago
Comment on attachment 221197 [details] [diff] [review]
look before leaping

r=wtc.

>     pk11KeyCallback *keycb = (pk11KeyCallback *) arg;
>+    if (!arg) {
>+        return SECFailure;
>+    }

I would test 'keycb' instead of 'arg' here.
Attachment #221197 - Flags: review+
CID 312
Whiteboard: [CID 312]
Crash Signature: [@ pk11_DoKeys]
You need to log in before you can comment on or make changes to this bug.