Slot reference leaks in signtool

RESOLVED FIXED in 3.5

Status

NSS
Tools
P2
normal
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
In signtool/util.c, function InitCrypto() calls PK11_GetInternalSlot()
but does not free the slot reference.

This function also calls PK11_GetInternalKeySlot() and does not free
that slot reference either.
(Assignee)

Comment 1

16 years ago
In signtool/certgen.c, function install_cert() calls
PK11_GetInternalKeySlot() but does not free the slot
reference.

    if( newSlot == PK11_GetInternalKeySlot() ) {
    
Bob, can this be replaced by the following?

    if( PK11_IsInternal(newSlot) ) {

Comment 2

16 years ago
Yes, your code is preferable.
(Assignee)

Comment 3

16 years ago
Changed the QA contact to Bishakha.
QA Contact: sonja.mirtitsch → bishakhabanerjee
(Assignee)

Comment 4

16 years ago
Created attachment 82968 [details] [diff] [review]
Proposed patch
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → 3.5
(Assignee)

Comment 5

16 years ago
Created attachment 82987 [details] [diff] [review]
Proposed patch

Bob told me that a block of code in install_dir() (in
signtool/certgen.c) is now obsolete and should be deleted.
After I did that, I noticed that the check for slot == NULL
looks like a typo for newSlot == NULL.	If that's correct,
then 'slot' is an unused function parameter and can be
deleted.

Bob also said that newSlot needs to be freed.

So this is what this new patch does.
Attachment #82968 - Attachment is obsolete: true

Comment 6

16 years ago
Looks correct to me, but since I directed the change, it's probably a good idea
to get a second opinion. ian?

Comment 7

16 years ago
Comment on attachment 82987 [details] [diff] [review]
Proposed patch


looks good.  Nice to remove another faulty attempt to set user trust ;)
Attachment #82987 - Flags: review+
(Assignee)

Comment 8

16 years ago
Thanks for the code reviews, Bob and Ian.

I checked the patch into the tip of NSS.  Marked the bug fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Component: Libraries → Tools
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.