281761 - VFY_CreateContext passes wrong key object to DecryptSigBlock

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Julien Pierre]

Comment 1

[Julien Pierre]

secvfy.c contains the following code :

	case rsaKey:
	    cx->type = VFY_RSA;
	    cx->key = SECKEY_CopyPublicKey(key); /* extra safety precautions */
	    if (sig) {
		int hashid = 0;
	    	rv = DecryptSigBlock(&hashid, &cx->digest[0], 
						key, sig, (char*)wincx);
		cx->alg = hashid;
	    } else {
		rv = decodeSigAlg(algid,&cx->alg);
	    }

This is incorrect. Why make a copy of the key object by precaution, and then
pass the original key object to DecryptSigBlock ?

The consequences of this mistake is that the original key object gets modified
underneath (the PKCS11 slot and object id get set) on the input key, rather than
the copied key .

Comment 2

[Julien Pierre]

Attachment: minor fixes

1) pass the correct publickey to DecryptSigBlock from VFY_CreateContext

Also, some minor code cleanup :
2) in DecryptSigBlock, remove unneeded dsig variable, and
allocation/deallocation of it

3) change type of hashid to SECOidTag instead of int
4) initialize hashid to SEC_OID_UNKNOWN, in case DecryptSigBlock fails . 

(the last two problems occurred twice in the file)

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 173933 [details] [diff] [review]
minor fixes

r=nelson
How I wish that NSS functions would routinely declare suppsedly-unmodified
arguments with const.  If NSS did that, the compiler would have found this bug
long ago.

Comment 4

[Julien Pierre]

Thanks, Nelson.

I checked this patch in to the tip for NSS 3.10 .

Checking in secvfy.c;
/cvsroot/mozilla/security/nss/lib/cryptohi/secvfy.c,v  <--  secvfy.c
new revision: 1.13; previous revision: 1.12