Closed
Bug 174188
Opened 22 years ago
Closed 22 years ago
pp crash printing certificate request generated by certutil
Categories
(NSS :: Tools, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.7
People
(Reporter: nelson, Assigned: wtc)
Details
Attachments
(2 files)
880 bytes,
patch
|
Details | Diff | Splinter Review | |
740 bytes,
text/plain
|
Details |
I created a certificate request using certutil (from NSS 3.3) with a command like this: certutil -d JAR -R -o JAR/req -a -s "CN=object signing cert, O=xxx.xxx" Then I tried to print out the certificate request using NSS's "pretty print" program pp (from NSS 3.6). pp printed the version number, the requested subject name, and the requested public key, and then crashed when it tried to print out the requests set of attributes (which should have been an empty set). This was repeatable. The crash was in the function SECU_PrintCertificateRequest. That function calls SEC_ASN1DecodeItem to decode the request, which succeeds. The result is a CERTCertificateRequest struct with 4 members, the fourth of which, cr->attributes, is NULL. The code in SECU_PrintCertificateRequest clearly doesn't ever expect to see it be null. It would be easy to fix this code to check for null pointers and avoid the crash. But the question is: does this bug reveal another bug, in the DER decoder? And, is the request perhaps encoded incorrectly? When encoding the cert request for a NULL list of attributes, as certutil does, CERT_CreateCertificateRequest allocates a small array of pointers to SECItems, and cr->attributes points to that array. The first SECItem pointer in the array is then set to NULL. Apparently, the author of SECU_PrintCertificateRequest expected the decoder to return something similar. But it doesn't. Should it? I suspect there are more bugs here than the simple lack of NULL pointer checking in SECU_PrintCertificateRequest and secu_PrintAny. I'm cc'ing our current DER encoder/decoder expert for his opinion.
Reporter | ||
Comment 1•22 years ago
|
||
This patch stops the crashes in pp, and makes pp print the request signature. But does it fix the bug? or merely mask it?
Reporter | ||
Comment 2•22 years ago
|
||
One more comment. The pp command that crashed was pp -t certificate-request -i JAR/req -a
Comment 3•22 years ago
|
||
Nelson, can you try this with NSS 3.6 or the tip ? This should use the new decoder which might have a different behavior. I'll answer your other questions shortly.
Reporter | ||
Comment 4•22 years ago
|
||
Julien, the crash occurs using the NSS 3.6 version of pp and the libraries. The cert request was generated wtih NSS 3.3, but the crash occurs when trying to pretty print it with NSS 3.6. I can regenerate the request using NSS 3.6, if you think that would be helpful.
Comment 5•22 years ago
|
||
Nelson, I think it's worth a try to do the cert request again with 3.6 , though most likely it won't change the outcome.
Reporter | ||
Comment 6•22 years ago
|
||
Reporter | ||
Comment 7•22 years ago
|
||
Patch checked in. pp doesn't crash any more. The remaining questions are: a) is the cert request correctly encoded, and b) is the decoder behaving properly when it returns NULL in cr->attributes? Or should cr->attributes point to a NULL pointer? (or ??)
Status: NEW → ASSIGNED
Reporter | ||
Comment 8•22 years ago
|
||
I now believe there IS a bug in SEC_ASN1DecodeItem revealed by this crash. At Julien's suggestion, I tried using SEC_QuickDERDecodeItem instead of SEC_ASN1DecodeItem in SECU_PrintCertificateRequest(), and the result was different. The cr->attributes pointer was not NULL then, and it correctly pointed to a NULL pointer. I will change SECU_PrintCertificateRequest to use SEC_QuickDERDecodeItem permanently, but someone needs to fix SEC_ASN1DecodeItem. I will change the subject of this bug to show that there IS a bug in SEC_ASN1DecodeItem.
Summary: pp crash implies possible DER en/decoder error (?) → SEC_ASN1DecodeItem bug revealed by pp crash
Reporter | ||
Comment 9•22 years ago
|
||
I'm marking this bug fixed, because pp no longer crashes and displays correct results. I will file a separate bug about SEC_ASN1DecodeItem.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Priority: -- → P1
Resolution: --- → FIXED
Summary: SEC_ASN1DecodeItem bug revealed by pp crash → pp crash printing certificate request generated by certutil
Target Milestone: --- → 3.7
You need to log in
before you can comment on or make changes to this bug.
Description
•