Closed Bug 315994 Opened 15 years ago Closed 15 years ago

pwdecrypt crashes and leaks memory

Categories

(NSS :: Tools, defect, P3)

3.10
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nelson, Assigned: nelson)

Details

Attachments

(1 file, 1 obsolete file)

pwdecrypt crashes when used to print a mozilla encrypted password file,
and it leaks memory for every line of encrypted text. 
This is not exploitable, and pwdecrypt is not part of the NSS release.

Prior to calling PK11SDR_Decrypt, pwdecrypt allocates a buffer that is
larger than the largest possible output, so that there will be room to 
add a trailing null character onto the end of the result.  The address 
of this buffer is stored in the SECItem "result".  But PK11SDR_Decrypt
always allocates its own result buffer, and stores the address of its
buffer in result, causing the previously allocated buffer to be leaked.

PK11SDR_Decrypt allocates a result buffer that is exactly big enough 
to hold the unpadded result, and not one byte longer.  So, after it
returns, when pwdecrypt stores a null character one byte past the end
of the result, it is overrunning the end of the result buffer in the 
heap.  This leads to a crash when the result buffer is freed.  

I have a patch that fixes these problems.
Attached patch patch v1Splinter Review
Julilen and/or Bob, please review.
Attachment #202623 - Flags: superreview?(rrelyea)
Attachment #202623 - Flags: review?(julien.pierre.bugs)
Comment on attachment 202623 [details] [diff] [review]
patch v1

This patch looks OK and I was able to run the program successfully.

However, the C++ comment (//) needs to be replaced with a C comment . r+ with this change .
Attachment #202623 - Flags: review?(julien.pierre.bugs) → review+
I ran this program with lots of threads on my browser DB with lots of certs. I couldn't get it to crash.
Assignee: nelson → julien.pierre.bugs
Status: NEW → ASSIGNED
Comment on attachment 202627 [details] [diff] [review]
test program stressing PK11_ListCerts in multithread mode

Woops. I attached this file to the wrong bug. This was meant for bug 79547 .
Attachment #202627 - Attachment is obsolete: true
Comment on attachment 202623 [details] [diff] [review]
patch v1

r+ with julien's cavaet about the comment.
Attachment #202623 - Flags: superreview?(rrelyea) → superreview+
I accidentally took this bug last night when I attached the wrong patch. Reassigning to Nelson.
Assignee: julien.pierre.bugs → nelson
Status: ASSIGNED → NEW
Checking in pwdecrypt.c;
/cvsroot/mozilla/security/nss/cmd/pwdecrypt/pwdecrypt.c,v  <--  pwdecrypt.c
new revision: 1.4; previous revision: 1.3
Status: NEW → RESOLVED
Closed: 15 years ago
Priority: -- → P3
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.