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

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[timeless]

Comment 1

[timeless]

Attachment: look before leaping

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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".  

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

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;
>+    }

Comment 4

[Alexei Volkov]

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

Comment 5

[Wan-Teh Chang]

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.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

CID 312