CERT_OpenCertDB() leaks the filename returned from the filename callback.
Ian, is there a duplicate for this bug?
Assignee: jgmyers → mcgreer
Status: ASSIGNED → NEW
Target Milestone: --- → 3.1
Created attachment 15933 [details] [diff] [review] Revised patch. certDBFilenameCallback needs to strdup the filename string.
This bug is similar to, but not a duplicate of, bug #39476. Comments on the revised patch (id=15933): 1. SEC_OpenPermCertDB (called by CERT_OpenCertDB) should free certdbname before it returns. 2. certDBFilenameCallback needs to duplicate the filename string so that its return value can be freed by SEC_OpenPermCertDB. Please review the patch. Thanks.
this looks fine to me. Thanks Wan-Teh.
I checked in the fix on the tip. /cvsroot/mozilla/security/nss/lib/certdb/pcertdb.c, revision 1.4
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
test_cdb_name_cb() in mailnews/mime/src/mimefilt.cpp needs to be changed to return the cert filename in malloced memory. Same thing for CertDBFilenameCallback() in security/nss/lib/certdb/pcertdb.c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
mailnews/mime/src/mimefilt.cpp is not part of NSS. My checkin already included the change you suggested for certDBFilenameCallback() in security/nss/lib/certdb/pcertdb.c.
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago → 18 years ago
Resolution: --- → FIXED
test_kdb_name_cb() in mailnews/mime/src/mimefilt.cpp also needs the same change.
Do any server products that use NSS also need similar changes?
Possibly. This needs to be mentioned in the release notes.
Are CERT_OpenCertDB() and SECKEY_OpenKeyDB() typically only called once in an application session? If so, I'd like to propose that we back out the PORT_Free() calls from these two functions so that we do not break existing filename callback functions that do not return their results in malloc'ed memory. On the other hand, since NSS 3.1 is likely to be the first 3.x release that most products will pick up, it might be sufficient to mention this change in the release notes.
SECKEY_OpenKeyDB() already had a PORT_Free() call for the failure case, so any existing filename callback function for that function which did not return its results in malloc'ed memory is already broken. All of the CERT_OpenCertDB() calls from adminsdk use functions which return malloc'ed memory.
> SECKEY_OpenKeyDB() already had a PORT_Free() call for > the failure case, so any existing filename callback function > for that function which did not return its results in > malloc'ed memory is already broken. Is it possible that the failure case code has never been exercised?
No, because the server I work on ran into it extremely early.
You need to log in before you can comment on or make changes to this bug.