587421 - PK11_PBEKeyGen leaks param because it checks type too late

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[timeless]

1346 	PK11_PBEKeyGen(PK11SlotInfo *slot, SECAlgorithmID *algid, SECItem *pwitem,
1356 	    pbeAlg = SECOID_GetAlgorithmTag(algid);
1359 	    if (sec_pkcs5_is_algorithm_v2_pkcs5_algorithm(pbeAlg)) {
1369 		pbeAlg = SECOID_GetAlgorithmTag(&pbeV2_param->pbeAlgId);
1370 		param = PK11_ParamFromAlgid(&pbeV2_param->pbeAlgId);
1377 	    } else {
1378 		param = PK11_ParamFromAlgid(algid);
1379 	    }

1384 	    type = PK11_AlgtagToMechanism(pbeAlg);	
1385 	    if (type == CKM_INVALID_MECHANISM) {
1387 		return NULL;

Comment 1

[timeless]

Attachment: patch

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 466133 [details] [diff] [review]
patch

Review requests for patches to pk11wrap and softoken should go to rrelyea

Comment 3

[Robert Relyea]

Comment on attachment 466133 [details] [diff] [review]
patch

r- this is hardly the way NSS normally deals with this. A more NSS-like patch forthcoming.

Comment 4

[Robert Relyea]

Attachment: Fix leak if type is invalid

Other reviews welcome (including timeless).

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 473624 [details] [diff] [review]
Fix leak if type is invalid

add'l r=nelson.  
It isn't necessary to check the argument to SECITEM_ZfreeItem for null 
before calling it.  It will do nothing if the argument is NULL.

Comment 6

[Elio Maldonado]

Comment on attachment 473624 [details] [diff] [review]
Fix leak if type is invalid

r+ emaldona

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Bug 587421: PK11_PBEKeyGen leaks param because it checks type too late
Patch contributed by Bob Relyea <[e-mail]>, r=nelson

Checking in pk11wrap/pk11pbe.c; new revision: 1.26; previous revision: 1.25