Closed Bug 315994 Opened 15 years ago Closed 15 years ago
pwdecrypt crashes and leaks memory
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.
Julilen and/or Bob, please review.
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.