Closed
Bug 314187
Opened 19 years ago
Closed 18 years ago
NSS PK11 Module: certificates with unknown AlgorythmIdentifiers make ThunderBird crashed
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.4
People
(Reporter: sergey.yakovlev, Assigned: alvolkov.bgs)
Details
Attachments
(1 file)
977 bytes,
patch
|
nelson
:
review+
julien.pierre
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; ru-RU; rv:1.7.5) Gecko/20041108 Firefox/1.0 Build Identifier: Thunderbird 1.0.6 (20050716) I tried to use ThunderBird with our smartcards via PKCS#11 interface. For some reason (at first, due to Ukrainian laws) we have to use some cryptographic algorithms, which is not mentioned in PKCS#11 - such as Russian GOST 28147-89 block cipher and Ukrainian DSTU 4145-2002 digital sign. Our certificates are X.509 compatible, but also uses those algorithms and their specific OIDs. I set the user personal certificate successfully. But when I tried to sign e-mail message, ThunderBird crashed with exception. Debugger said, that error occures in function nss3.PK11_FortezzaHasKEA(). (This very surprised me, 'cause our smartcard readers are not Fortezza slots and they do not support KEA algorytms.) When I've looked in source code (latest, TB v1.5b2), I find that, really, this function may crash: PRBool PK11_FortezzaHasKEA(CERTCertificate *cert) { /* look at the subject and see if it is a KEA for MISSI key */ SECOidData *oid; if ((cert->trust == NULL) || ((cert->trust->sslFlags & CERTDB_USER) != CERTDB_USER)) { return PR_FALSE; } oid = SECOID_FindOID(&cert->subjectPublicKeyInfo.algorithm.algorithm); return (PRBool)((oid->offset == SEC_OID_MISSI_KEA_DSS_OLD) || (oid->offset == SEC_OID_MISSI_KEA_DSS) || (oid->offset == SEC_OID_MISSI_KEA)) ; } Function SECOID_FindOID() does not recognize given OID, returns NULL and sets error SEC_ERROR_UNRECOGNIZED_OID. But PK11_FortezzaHasKEA() does not check, if error occures!.. So program tries to read memory at NULL address and crashes. From this we have two points: 1. PK11_FortezzaHasKEA() has an error. 2. NSS looks for KEA algorythms when device has said (via PKCS#11 function C_GetMechanismList) that it does not support them. So, a general question is: Can NSS support cryptographic mechanisms except described/mentioned in PKCS#11 (or mentioned as "vendor defined")? May be, there is some unobvious way to use them? Reproducible: Always
Updated•18 years ago
|
Assignee: wtchang → nobody
QA Contact: jason.m.reid → libraries
Updated•18 years ago
|
Assignee: nobody → alexei.volkov.bugs
Severity: major → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Target Milestone: --- → 3.11.4
Version: unspecified → 3.9
Comment 1•18 years ago
|
||
> 1. PK11_FortezzaHasKEA() has an error. Yes. > 2. NSS looks for KEA algorythms when device has said (via PKCS#11 function > C_GetMechanismList) that it does not support them. This is certificate processing code. We don't have separate certificate processing code for tokens that do support fortezza and for tokens that don't. We have one body of certificate processing code and it handles both cases. This code is trying to answer the question: does this certificate have a fortezza public key? That's a legitimate question to ask regardless of the source of the certificate. So the only issue here is that PK11_FortezzaHasKEA() has an error.
Assignee | ||
Comment 2•18 years ago
|
||
Attachment #237013 -
Flags: review?(nelson)
Comment 3•18 years ago
|
||
Comment on attachment 237013 [details] [diff] [review] check for returned OID r=nelsonb. I'm sensing Deja-vu over this patch. Is this bug a duplicate of another one?
Attachment #237013 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 4•18 years ago
|
||
Comment on attachment 237013 [details] [diff] [review] check for returned OID second review for 3.11 branch
Attachment #237013 -
Flags: superreview?(julien.pierre.bugs)
Assignee | ||
Comment 5•18 years ago
|
||
Nelson, bugzilla search finds only one bug. We fixed plenty of similar bugs reported by coverity. No wonder this one looks similar.
Updated•18 years ago
|
Attachment #237013 -
Flags: superreview?(julien.pierre.bugs) → superreview+
Assignee | ||
Comment 6•18 years ago
|
||
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v <-- pk11cert.c new revision: 1.154; previous revision: 1.153
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•