340018 - Coverity 222 & 223, NSSCKFWC_InitToken passes NULL to nssCKFWToken_Destroy

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

In NSSCKFWC_InitToken, there are numerous error paths that set variable 
"error" to some non-zero value and then go to loser.   Many of these paths 
have the effect that variable fwToken is NULL when label loser is reached.

At label loser, a switch on the variable "error" will cause fwToken to be
passed to nssCKFWToken_Destroy, which will dereference it.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Coverity CID 220 & CID 221
This same problem is present in NSSCKFWC_GetTokenInfo.

Coverity CID 218 and CID 219
Same problem in  NSSCKFWC_GetMechanismList

Coverity CID 216 & CID 217
Same problem in  NSSCKFWC_GetMechanismInfo

I think the solution in all these functiosn is to change one line of 
duplicated code, as follows:

646  	 loser:
647  	  switch( error ) {

At conditional (3): "error == 50" taking true path

648  	  case CKR_DEVICE_REMOVED:
649  	  case CKR_TOKEN_NOT_PRESENT:
650-  	    (void)nssCKFWToken_Destroy(fwToken);
650+ 	    if (fwToken) (void)nssCKFWToken_Destroy(fwToken);
651  	    break;

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

Bob, please review

Comment 3

[Alexei Volkov]

Comment on attachment 224832 [details] [diff] [review]
patch v1

r=alexei.volkov

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Don't call nssCKFWToken_Destroy with NULL. Bug 340018. r=alexei.volkov
Checking in wrap.c; new revision: 1.13.2.1; previous revision: 1.13
Checking in wrap.c; new revision: 1.15;     previous revision: 1.14

Comment 5

[Robert Relyea]

Comment on attachment 224832 [details] [diff] [review]
patch v1

r+