Closed Bug 414003 Opened 13 years ago Closed 13 years ago
Crash [@ CERT
_Decode Cert Package] sometimes with this testcase, that imports a certificate
I sometimes get a crash when importing this certificate with a testcase, that continuously imports that certificate. I'll attach that testcase, after this file has been attached. http://crash-stats.mozilla.com/report/index/9e0851ee-cb4a-11dc-b5ff-001a4bd43ed6 0 CERT_DecodeCertPackage mozilla/security/nss/lib/pkcs7/certread.c:474 1 nsNSSCertificateDB::getCertsFromPackage(PLArenaPool*, unsigned char*, unsigned int) mozilla/security/manager/ssl/src/nsNSSCertificateDB.cpp:266 2 nsNSSCertificateDB::ImportCertificates(unsigned char*, unsigned int, unsigned int, nsIInterfaceRequestor*) mozilla/security/manager/ssl/src/nsNSSCertificateDB.cpp:477 3 nsNSSCertificateDB::ImportCertsFromFile(nsISupports*, nsILocalFile*, unsigned int) mozilla/security/manager/ssl/src/nsNSSCertificateDB.cpp:1146 4 NS_InvokeByIndex_P mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101 5 JS_SuspendRequest 6 @0x2 It also seems to crash on branch. The crash can occur within 10 seconds, but it also can take 2 minutes, it seems to take a random amount of time.
To reproduce: - Download the testcase and certificate to your computer - Open the testcase, a file-picker pops up, choose the certificate file - Wait for the crash to happen.
Crash was caused by incorrectly formatted ASCII cert, -----END CERITIFICATE----- was broken, and because cl counter of content left to parse was not moved forward with cp pointer after -----BEGIN CERTIFICATE----- was found we were trying to parse behind the data of the cert. I changed conditions to (cl > sizeof(NS_CERT_TRAILER)-1) because sizeof returns also the trailing zero. (cl > sizeof(NS_CERT_TRAILER)) expect two additional chars behind. For mac/linux files where is not any CR char this would not pass, needlessly.
Assignee: kengert → honzab
Status: NEW → ASSIGNED
Attachment #315075 - Flags: review?(kengert)
The patch you attached is in NSS code, so I'm changing the component of this bug and assigning to default owners.
Assignee: honzab → nobody
Status: ASSIGNED → NEW
Component: Security: PSM → Libraries
Product: Core → NSS
QA Contact: psm → libraries
Version: Trunk → unspecified
Comment on attachment 315075 [details] [diff] [review] Fix Bob should review this code.
Attachment #315075 - Flags: review?(kengert) → review?(rrelyea)
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/security/nss/lib/pkcs7/certread.c&rev=1.11 CC'ing Nelson and Wan-Teh who have touched this code and hopefully can review it.
Thanks to Honza Bambas (:mayhemer) for identifying the cause of this bug. I've written an alternative patch that fixes ALL the incorrect usages of sizeof, and also eliminates duplicate copies of constant strings.
Comment on attachment 315142 [details] [diff] [review] Fix all the incorrect usages of sizeof r=alexei
Attachment #315142 - Flags: superreview?(alexei.volkov.bugs) → superreview+
Bob, Do we want to get this into 3.12 or wait for 3.12.1 ?
Priority: -- → P2
Target Milestone: --- → 3.12
Comment on attachment 315142 [details] [diff] [review] Fix all the incorrect usages of sizeof 3.12.1 r+ rrelyea
Attachment #315142 - Flags: review?(kengert) → review+
Comment on attachment 315075 [details] [diff] [review] Fix obsoleted by nelson's patch. Good investigation work Honza bob
lib/pkcs7/certread.c; new revision: 1.12; previous revision: 1.11
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I'm not planning to put this into 3.12.0 Somebody holler if that's a problem.
Target Milestone: 3.12 → 3.12.1
Crash Signature: [@ CERT_DecodeCertPackage]
You need to log in before you can comment on or make changes to this bug.