Created attachment 299219 [details] 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.
Created attachment 299220 [details] testcase 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.
Created attachment 315075 [details] [diff] [review] Fix 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.
The patch you attached is in NSS code, so I'm changing the component of this bug and assigning to default owners.
Comment on attachment 315075 [details] [diff] [review] Fix Bob should review this code.
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.
Created attachment 315142 [details] [diff] [review] Fix all the incorrect usages of sizeof 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
Bob, Do we want to get this into 3.12 or wait for 3.12.1 ?
Comment on attachment 315142 [details] [diff] [review] Fix all the incorrect usages of sizeof 3.12.1 r+ rrelyea
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
I'm not planning to put this into 3.12.0 Somebody holler if that's a problem.