Closed
Bug 86981
Opened 23 years ago
Closed 23 years ago
cmsutil coredumps due to uninitialized auto variables in smime lib.
Categories
(NSS :: Libraries, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.3
People
(Reporter: mbar, Assigned: wtc)
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
cmsutil is dumping core in smime tests due to uninitialized auto variables in
cmsrecinfo.c. In the function NSS_CMSRecipientInfo_Create(), none of the auto
variables are initialized. The variable rv is used to indicate success or
failure at one point in the code, but the variable is only "explicitly set" in
the case of failure. Because of the way the code is written, a successful code
path can be taken, but the variable, not being initialized, can take on a value
indicating failure to the later code.
Please note that this is clearly a cross-platform bug.
All of the auto-storage variables, especially those used for branching decisions
should be initialized to a sensible value. Otherwise, the code is very liable
to intermittent errors. Please refer to the C standard's rules for auto-storage
variables and their initialized value.
Below you'll find code snippets from the pertinent sections of certdb.c,
cmsrecinfo.c, smime.sh (test driver), and the debugging output from the test
run. I have put in quite a few statements for debugging output in the C
modules. I've also included the pertinent section of the output from the test
run so the order in which the debugging statements are executed can be seen.
Please forward any questions.
Cheers,
Matthew
============================================================================
mozilla/security/nss/lib/smime/cmsrecinfo.c
============================================================================
NSSCMSRecipientInfo *
NSS_CMSRecipientInfo_Create(NSSCMSMessage *cmsg, CERTCertificate *cert)
{
NSSCMSRecipientInfo *ri;
void *mark;
SECOidTag certalgtag;
SECStatus rv;
NSSCMSRecipientEncryptedKey *rek;
NSSCMSOriginatorIdentifierOrKey *oiok;
unsigned long version;
SECItem *dummy;
PLArenaPool *poolp;
#if 1
fprintf( stderr, "NSS_CMSRecipientInfo_Create(NSSCMSMessage *cmsg=%p,
CERTCertificate *cert=%p)\n",
cmsg, cert );
#endif
poolp = cmsg->poolp;
mark = PORT_ArenaMark(poolp);
ri = (NSSCMSRecipientInfo *)PORT_ArenaZAlloc(poolp,
sizeof(NSSCMSRecipientInfo));
#if 1
fprintf( stderr, "1>> poolp=%p, ri=%p\n", poolp, ri );
#endif
if (ri == NULL)
goto loser;
ri->cmsg = cmsg;
ri->cert = CERT_DupCertificate(cert);
#if 1
fprintf( stderr, "2>> ri=%p, ri->cert=%p, ri->cmsg=%p\n", ri, ri->cert,
ri->cmsg );
#endif
if (ri->cert == NULL)
goto loser;
certalgtag =
SECOID_GetAlgorithmTag(&(cert->subjectPublicKeyInfo.algorithm));
switch (certalgtag) {
case SEC_OID_PKCS1_RSA_ENCRYPTION:
#if 1
fprintf( stderr, "2.1>> case SEC_OID_PKCS1_RSA_ENCRYPTION:\n" );
#endif
ri->recipientInfoType = NSSCMSRecipientInfoID_KeyTrans;
/* hardcoded issuerSN choice for now */
ri->ri.keyTransRecipientInfo.recipientIdentifier.identifierType =
NSSCMSRecipientID_IssuerSN;
#if 1
fprintf( stderr, "2.2>> poolp=%p, cert=%p\n", poolp, cert );
#endif
ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN =
CERT_GetCertIssuerAndSN(poolp, cert);
#if 1
fprintf( stderr, "2.3>>
ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN=%p\n",
ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN );
#endif
if (ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN == NULL) {
#if 1
fprintf( stderr, "2.4>> Going to failure\n" );
#endif
rv = SECFailure;
break;
}
#if 1
fprintf( stderr, "2.5.0>> SECFailure == %ld\n", SECFailure );
fprintf( stderr, "2.5.1>> Taking successful branch.\n" );
#endif
break;
case SEC_OID_MISSI_KEA_DSS_OLD:
#if 1
fprintf( stderr, "2.1>> case SEC_OID_MISSI_KEA_DSS_OLD:\n" );
#endif
case SEC_OID_MISSI_KEA_DSS:
#if 1
fprintf( stderr, "2.1>> case SEC_OID_MISSI_KEA_DSS:\n" );
#endif
case SEC_OID_MISSI_KEA:
#if 1
fprintf( stderr, "2.1>> case SEC_OID_MISSI_KEA:\n" );
#endif
/* backward compatibility - this is not really a keytrans operation */
ri->recipientInfoType = NSSCMSRecipientInfoID_KeyTrans;
/* hardcoded issuerSN choice for now */
ri->ri.keyTransRecipientInfo.recipientIdentifier.identifierType =
NSSCMSRecipientID_IssuerSN;
ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN =
CERT_GetCertIssuerAndSN(poolp, cert);
if (ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN == NULL) {
rv = SECFailure;
break;
}
break;
case SEC_OID_X942_DIFFIE_HELMAN_KEY: /* dh-public-number */
#if 1
fprintf( stderr, "2.1>> case SEC_OID_X942_DIFFIE_HELMAN_KEY: /*
dh-public-number */\n" );
#endif
/* a key agreement op */
ri->recipientInfoType = NSSCMSRecipientInfoID_KeyAgree;
if (ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN == NULL) {
rv = SECFailure;
break;
}
/* we do not support the case where multiple recipients
* share the same KeyAgreeRecipientInfo and have multiple
RecipientEncryptedKeys
* in this case, we would need to walk all the recipientInfos, take the
* ones that do KeyAgreement algorithms and join them, algorithm by algorithm
* Then, we'd generate ONE ukm and OriginatorIdentifierOrKey */
/* only epheremal-static Diffie-Hellman is supported for now
* this is the only form of key agreement that provides potential anonymity
* of the sender, plus we do not have to include certs in the message */
/* force single recipientEncryptedKey for now */
if ((rek = NSS_CMSRecipientEncryptedKey_Create(poolp)) == NULL) {
rv = SECFailure;
break;
}
/* hardcoded IssuerSN choice for now */
rek->recipientIdentifier.identifierType = NSSCMSKeyAgreeRecipientID_IssuerSN;
if ((rek->recipientIdentifier.id.issuerAndSN = CERT_GetCertIssuerAndSN(poolp,
cert)) == NULL) {
rv = SECFailure;
break;
}
oiok = &(ri->ri.keyAgreeRecipientInfo.originatorIdentifierOrKey);
/* see RFC2630 12.3.1.1 */
oiok->identifierType = NSSCMSOriginatorIDOrKey_OriginatorPublicKey;
rv = NSS_CMSArray_Add(poolp, (void
***)&ri->ri.keyAgreeRecipientInfo.recipientEncryptedKeys,
(void *)rek);
break;
default:
#if 1
fprintf( stderr, "2.1>> default:\n" );
#endif
/* other algorithms not supported yet */
/* NOTE that we do not support any KEK algorithm */
PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
rv = SECFailure;
break;
}
if (rv == SECFailure) {
#if 1
fprintf( stderr, "3>> rv==SECFailure\n" );
#endif
goto loser;
}
.
.
.
============================================================================
============================================================================
mozilla/security/nss/lib/certdb/certdb.c - Starting Line 1498
============================================================================
CERTIssuerAndSN *
CERT_GetCertIssuerAndSN(PRArenaPool *arena, CERTCertificate *cert)
{
CERTIssuerAndSN *result;
SECStatus rv;
#if 1
fprintf( stderr, "1.0.0>> CERT_GetCertIssuerAndSN(PRArenaPool *arena=%p,
CERTCertificate *cert=%p), cert->arena=%p\n",
arena , cert, cert->arena );
#endif
if ( arena == NULL ) {
arena = cert->arena;
}
result = (CERTIssuerAndSN*)PORT_ArenaZAlloc(arena, sizeof(*result));
#if 1
fprintf( stderr, "1.1.0>> result = PORT_ArenaZAlloc() = %p \n", result );
#endif
if (result == NULL) {
PORT_SetError (SEC_ERROR_NO_MEMORY);
return NULL;
}
#if 1
fprintf( stderr, "1.2.0>> (arena=%p, &result->derIssuer=%p,
&cert->derIssuer=%p)\n", arena, &result->derIssuer, &cert->derIssuer );
#endif
rv = SECITEM_CopyItem(arena, &result->derIssuer, &cert->derIssuer);
#if 1
fprintf( stderr, "1.2.1>> (arena=%p, &result->derIssuer=%p,
&cert->derIssuer=%p)\n", arena, &result->derIssuer, &cert->derIssuer );
fprintf( stderr, "1.3.0>> rv=%s \n", (rv==SECSuccess)?"OK":"Not OK" );
#endif
if (rv != SECSuccess)
return NULL;
rv = CERT_CopyName(arena, &result->issuer, &cert->issuer);
#if 1
fprintf( stderr, "1.4.0>> CERT_CopyName(arena=%p, &result->issuer=%p,
&cert->issuer=%p)\n", arena, &result->derIssuer, &cert->derIssuer );
fprintf( stderr, "1.5.0>> rv=%s \n", (rv==SECSuccess)?"OK":"Not OK" );
#endif
if (rv != SECSuccess)
return NULL;
rv = SECITEM_CopyItem(arena, &result->serialNumber, &cert->serialNumber);
#if 1
fprintf( stderr, "1.6.0>> SECITEM_CopyItem(arena=%p,
&result->serialNumber=%p, &cert->serialNumber=%p)\n", arena,
&result->serialNumber, &cert->serialNumber);
fprintf( stderr, "1.7.0>> rv=%s \n", (rv==SECSuccess)?"OK":"Not OK" );
#endif
if (rv != SECSuccess)
return NULL;
#if 1
fprintf( stderr, "1.8.0>> result=%p \n", result );
#endif
return result;
}
============================================================================
============================================================================
nss/tests/smime/smime.sh: added two lines at line 109 to stop after coredump.
============================================================================
.
.
.
107 cmsutil -E -r bob@bogus.com -i alice.txt -d ${R_ALICEDIR} -p nss -o
alice.env
108 html_msg $? 0 "Create Enveloped Data Alice" "."
109+ # MDB vvv
110+ exit
.
.
.
============================================================================
Output from test run of nss/tests/smime/smime.sh
============================================================================
.
.
.
imported certificate
1.0.0>> CERT_GetCertIssuerAndSN(PRArenaPool *arena=1009b078, CERTCertificate
*cert=10098940), cert->arena=100988c0
1.1.0>> result = PORT_ArenaZAlloc() = 10097238
1.2.0>> (arena=1009b078, &result->derIssuer=10097238, &cert->derIssuer=10098988)
1.2.1>> (arena=1009b078, &result->derIssuer=10097238, &cert->derIssuer=10098988)
1.3.0>> rv=OK
1.4.0>> CERT_CopyName(arena=1009b078, &result->issuer=10097238,
&cert->issuer=10098988)
1.5.0>> rv=OK
1.6.0>> SECITEM_CopyItem(arena=1009b078, &result->serialNumber=1009724c,
&cert->serialNumber=100989c4)
1.7.0>> rv=OK
1.8.0>> result=10097238
created signed-date message
cmsg [1009ac68]
arena [10097068]
password [nss]
input len [158]
44 61 74 65 3a 20 57 65 64 2c 20 32 30 20 53 65 70 20 32 30 30 30 20 30 30 3a 30
30 3a 30 31 20 2d 30 37 30
30 20 28 50 44 54 29 a 46 72 6f 6d 3a 20 61 6c 69 63 65 40 62 6f 67 75 73 2e 63
6f 6d a 53 75 62 6a 65
63 74 3a 20 6d 65 73 73 61 67 65 20 41 6c 69 63 65 20 2d 2d 3e 20 42 6f 62 a 54
6f 3a 20 62 6f 62 40 62
6f 67 75 73 2e 63 6f 6d a a 54 68 69 73 20 69 73 20 61 20 74 65 73 74 20 6d 65
73 73 61 67 65 20 66 72
6f 6d 20 41 6c 69 63 65 20 74 6f 20 42 6f 62 2e a encoding passed
wrote to file
smime.sh: Create Signature Alice . PASSED
cmsutil -D -i alice.sig -d ../bobdir -o alice.data1
starting program
parsed command line
received commands
NSS has been initialized.
Got default certdb
DECODE
smime.sh: Decode Alice's Signature . PASSED
diff alice.txt alice.data1
smime.sh: Compare Decoded Signature and Original . PASSED
smime.sh: Enveloped Data Tests ------------------------------
cmsutil -E -r bob@bogus.com -i alice.txt -d ../alicedir -p nss \
-o alice.env
starting program
parsed command line
received commands
NSS has been initialized.
Got default certdb
NSS_CMSRecipientInfo_Create(NSSCMSMessage *cmsg=100a4eb0, CERTCertificate
*cert=100a4690)
1>> poolp=1009b078, ri=100a4ff0
2>> ri=100a4ff0, ri->cert=100a4690, ri->cmsg=100a4eb0
2.1>> case SEC_OID_PKCS1_RSA_ENCRYPTION:
2.2>> poolp=1009b078, cert=100a4690
1.0.0>> CERT_GetCertIssuerAndSN(PRArenaPool *arena=1009b078, CERTCertificate
*cert=100a4690), cert->arena=1009b008
1.1.0>> result = PORT_ArenaZAlloc() = 100a5050
1.2.0>> (arena=1009b078, &result->derIssuer=100a5050, &cert->derIssuer=100a46d8)
1.2.1>> (arena=1009b078, &result->derIssuer=100a5050, &cert->derIssuer=100a46d8)
1.3.0>> rv=OK
1.4.0>> CERT_CopyName(arena=1009b078, &result->issuer=100a5050,
&cert->issuer=100a46d8)
1.5.0>> rv=OK
1.6.0>> SECITEM_CopyItem(arena=1009b078, &result->serialNumber=100a5064,
&cert->serialNumber=100a4714)
1.7.0>> rv=OK
1.8.0>> result=100a5050
2.3>> ri->ri.keyTransRecipientInfo.recipientIdentifier.id.issuerAndSN=100a5050
2.5.0>> SECFailure == -1
2.5.1>> Taking successful branch.
3>> rv==SECFailure
ERROR: cannot create CMS recipientInfo object.
cmsutil: problem enveloping: Certificate extension not found.
cmsg [0]
arena [10097030]
password [nss]
smime_main[19]: 279131 Memory fault(coredump)
smime.sh: Create Enveloped Data Alice . FAILED
Updated•23 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.2.2
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Kirk, could you please review my patch? Thank you.
Comment 3•23 years ago
|
||
SECStatus
NSS_CMSRecipientInfo_WrapBulkKey(NSSCMSRecipientInfo *ri, PK11SymKey *bulkkey,
SECOidTag bulkalgtag)
{
CERTCertificate *cert;
SECOidTag certalgtag;
SECStatus rv; <--- line 277
I believe this spot in the same file needs it too.
For example the first case (SEC_OID_PKCS1_RSA_ENCRYPTION) might not
set rv. Also the SEC_OID_X942_DIFFIE_HELMAN_KEY case.
Otherwise, your change gets my approval.
Good work Matthew and Wan-Teh!
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Comment 5•23 years ago
|
||
I checked in the revised patch on the trunk of NSS.
This fix will be in NSS 3.3.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Component: Tools → Libraries
Resolution: --- → FIXED
Target Milestone: 3.2.2 → 3.3
You need to log in
before you can comment on or make changes to this bug.
Description
•