354423 - modutil -changepw command crashes in debug builds

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

The reason for the crash is this:

746     if(newpw) {
747         memset(newpw, 0, strlen(newpw));
748         PORT_Free(newpw);
749     }
750     if(newpw2) {
751         memset(newpw2, 0, strlen(newpw));
                                         ^^
should be newpw2
 
752         PORT_Free(newpw2);
753     }

The patch is to insert the digit "2" in line 751.
Can I get a review on that patch here?

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Here's a second observation that I might as well attach to this same bug

When NSS_STRICT_SHUTDOWN is set to 1,  the command 
   modutil.exe -dbdir FIPS -changepw "NSS FIPS 140-2 Certificate DB"
crashes in NSS_Shutdown due to a reference leak.
This is a separate problem from the above corrupt heap issue, but we might
as well kill two modutil bugs with one stone.

Bob, will you look at that?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

Alexei, please review.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

I have filed a separate bug for the problem described in comment 1.
That bug is now bug 359331

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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

Bob, please SR

Comment 5

[Robert Relyea]

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

r+, though wouldn't PORT_ZFree be more appropriate?

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Checked in

> Bug 354423.  Get the right string length. R=alexei.volkov,rrelyea

Checking in pk11.c; new revision: 1.21.28.2; previous revision: 1.21.28.1
Checking in pk11.c; new revision: 1.24; previous revision: 1.23