330056 - seckey_put_private_key leaks memory

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Wyllys Ingersoll]

User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8) Gecko/20051122 Firefox/1.5
Build Identifier: 3.10

seckey_put_private_key (from security/nss/lib/softoken/keydb.c) leaks
memory allocated for the dbkey structure.

"permarena" is allocated locally but it is not freed unless the status != SECSuccess.  It should be freed always because it is never used or referenced again.





Reproducible: Always

Steps to Reproduce:
1. Call PK11_GenerateKeyPair (with IsPermanent== TRUE, IsSensitive == TRUE)
2. Call PK11_DeleteTokenPublicKey()
3. Call PK11_DeleteTokenPrivateKey()

Check for leaks.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Appears to have been there in the original NSS 3.0 code.
Thanks for the report, Wyllys.

Comment 2

[Alexei Volkov]

Attachment: free permarena regardless rv value

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 226727 [details] [diff] [review]
free permarena regardless rv value

r=nelson

Comment 4

[Alexei Volkov]

Comment on attachment 226727 [details] [diff] [review]
free permarena regardless rv value

Fix is in softoken. I think this is important to have it fix in 3.11.x release. Asking for second review.

Comment 5

[Wan-Teh Chang]

Comment on attachment 226727 [details] [diff] [review]
free permarena regardless rv value

r=wtc.  You can check in this patch on the NSS_3_11_BRANCH.

The variable name 'permarena' is misleading in this function.
It should be renamed 'arena'.  The variable 'temparena' should
be removed, with the following code:

    if(temparena != NULL)
        PORT_FreeArena(temparena, PR_TRUE);

replaced by:

    if(epki != NULL)
        PORT_FreeArena(epki->arena, PR_TRUE);

Comment 6

[Alexei Volkov]

tip:
/cvsroot/mozilla/security/nss/lib/softoken/keydb.c,v  <--  keydb.c
new revision: 1.48; previous revision: 1.47

3.11 branch:
/cvsroot/mozilla/security/nss/lib/softoken/keydb.c,v  <--  keydb.c
new revision: 1.40.2.6; previous revision: 1.40.2.5

Comment 7

[Wan-Teh Chang]

Please attach the patch you actually checked in.

Comment 8

[Alexei Volkov]

Attachment: Integrated patch