From d54a1f812ae23ec11d2af6ed93ba1a11609421a8 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Mon, 14 Jan 2019 10:35:25 -0700 Subject: [PATCH] Bug 1507135 - Add additional null checks to CMS message functions r=mt Differential review: https://phabricator.services.mozilla.com//D16488 --HG-- branch : NSS_3_36_BRANCH extra : transplant_source : 1%02%80%21%BE%C8B%D5%21%D7%0CR%00%ED%B6%EA%84a%FA%23 --- lib/smime/cmsmessage.c | 69 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/lib/smime/cmsmessage.c b/lib/smime/cmsmessage.c index 27d1256ec..f41a432b1 100644 --- a/lib/smime/cmsmessage.c +++ b/lib/smime/cmsmessage.c @@ -29,8 +29,9 @@ NSS_CMSMessage_Create(PLArenaPool *poolp) if (poolp == NULL) { poolp = PORT_NewArena(1024); /* XXX what is right value? */ - if (poolp == NULL) + if (poolp == NULL) { return NULL; + } poolp_is_ours = PR_TRUE; } @@ -44,8 +45,9 @@ NSS_CMSMessage_Create(PLArenaPool *poolp) if (mark) { PORT_ArenaRelease(poolp, mark); } - } else + } else { PORT_FreeArena(poolp, PR_FALSE); + } return NULL; } @@ -53,8 +55,9 @@ NSS_CMSMessage_Create(PLArenaPool *poolp) cmsg->poolp_is_ours = poolp_is_ours; cmsg->refCount = 1; - if (mark) + if (mark) { PORT_ArenaUnmark(poolp, mark); + } return cmsg; } @@ -73,8 +76,13 @@ NSS_CMSMessage_SetEncodingParams(NSSCMSMessage *cmsg, NSSCMSGetDecryptKeyCallback decrypt_key_cb, void *decrypt_key_cb_arg, SECAlgorithmID **detached_digestalgs, SECItem **detached_digests) { - if (pwfn) + if (cmsg == NULL) { + return; + } + if (pwfn) { PK11_SetPasswordFunc(pwfn); + } + cmsg->pwfn_arg = pwfn_arg; cmsg->decrypt_key_cb = decrypt_key_cb; cmsg->decrypt_key_cb_arg = decrypt_key_cb_arg; @@ -89,18 +97,21 @@ void NSS_CMSMessage_Destroy(NSSCMSMessage *cmsg) { PORT_Assert(cmsg->refCount > 0); - if (cmsg->refCount <= 0) /* oops */ + if (cmsg->refCount <= 0) { /* oops */ return; + } cmsg->refCount--; /* thread safety? */ - if (cmsg->refCount > 0) + if (cmsg->refCount > 0) { return; + } NSS_CMSContentInfo_Destroy(&(cmsg->contentInfo)); /* if poolp is not NULL, cmsg is the owner of its arena */ - if (cmsg->poolp_is_ours) + if (cmsg->poolp_is_ours) { PORT_FreeArena(cmsg->poolp, PR_FALSE); /* XXX clear it? */ + } } /* @@ -112,8 +123,9 @@ NSS_CMSMessage_Destroy(NSSCMSMessage *cmsg) NSSCMSMessage * NSS_CMSMessage_Copy(NSSCMSMessage *cmsg) { - if (cmsg == NULL) + if (cmsg == NULL) { return NULL; + } PORT_Assert(cmsg->refCount > 0); @@ -127,6 +139,10 @@ NSS_CMSMessage_Copy(NSSCMSMessage *cmsg) PLArenaPool * NSS_CMSMessage_GetArena(NSSCMSMessage *cmsg) { + if (cmsg == NULL) { + return NULL; + } + return cmsg->poolp; } @@ -136,6 +152,10 @@ NSS_CMSMessage_GetArena(NSSCMSMessage *cmsg) NSSCMSContentInfo * NSS_CMSMessage_GetContentInfo(NSSCMSMessage *cmsg) { + if (cmsg == NULL) { + return NULL; + } + return &(cmsg->contentInfo); } @@ -147,6 +167,10 @@ NSS_CMSMessage_GetContentInfo(NSSCMSMessage *cmsg) SECItem * NSS_CMSMessage_GetContent(NSSCMSMessage *cmsg) { + if (cmsg == NULL) { + return NULL; + } + /* this is a shortcut */ NSSCMSContentInfo *cinfo = NSS_CMSMessage_GetContentInfo(cmsg); SECItem *pItem = NSS_CMSContentInfo_GetInnerContent(cinfo); @@ -164,6 +188,10 @@ NSS_CMSMessage_ContentLevelCount(NSSCMSMessage *cmsg) int count = 0; NSSCMSContentInfo *cinfo; + if (cmsg == NULL) { + return 0; + } + /* walk down the chain of contentinfos */ for (cinfo = &(cmsg->contentInfo); cinfo != NULL;) { count++; @@ -183,6 +211,10 @@ NSS_CMSMessage_ContentLevel(NSSCMSMessage *cmsg, int n) int count = 0; NSSCMSContentInfo *cinfo; + if (cmsg == NULL) { + return NULL; + } + /* walk down the chain of contentinfos */ for (cinfo = &(cmsg->contentInfo); cinfo != NULL && count < n; cinfo = NSS_CMSContentInfo_GetChildContentInfo(cinfo)) { @@ -200,6 +232,10 @@ NSS_CMSMessage_ContainsCertsOrCrls(NSSCMSMessage *cmsg) { NSSCMSContentInfo *cinfo; + if (cmsg == NULL) { + return PR_FALSE; + } + /* descend into CMS message */ for (cinfo = &(cmsg->contentInfo); cinfo != NULL; cinfo = NSS_CMSContentInfo_GetChildContentInfo(cinfo)) { @@ -221,6 +257,10 @@ NSS_CMSMessage_IsEncrypted(NSSCMSMessage *cmsg) { NSSCMSContentInfo *cinfo; + if (cmsg == NULL) { + return PR_FALSE; + } + /* walk down the chain of contentinfos */ for (cinfo = &(cmsg->contentInfo); cinfo != NULL; cinfo = NSS_CMSContentInfo_GetChildContentInfo(cinfo)) { @@ -251,13 +291,21 @@ NSS_CMSMessage_IsSigned(NSSCMSMessage *cmsg) { NSSCMSContentInfo *cinfo; + if (cmsg == NULL) { + return PR_FALSE; + } + /* walk down the chain of contentinfos */ for (cinfo = &(cmsg->contentInfo); cinfo != NULL; cinfo = NSS_CMSContentInfo_GetChildContentInfo(cinfo)) { switch (NSS_CMSContentInfo_GetContentTypeTag(cinfo)) { case SEC_OID_PKCS7_SIGNED_DATA: - if (!NSS_CMSArray_IsEmpty((void **)cinfo->content.signedData->signerInfos)) + if (cinfo->content.signedData == NULL) { + return PR_FALSE; + } + if (!NSS_CMSArray_IsEmpty((void **)cinfo->content.signedData->signerInfos)) { return PR_TRUE; + } break; default: /* callback here for generic wrappers? */ @@ -278,8 +326,9 @@ NSS_CMSMessage_IsContentEmpty(NSSCMSMessage *cmsg, unsigned int minLen) { SECItem *item = NULL; - if (cmsg == NULL) + if (cmsg == NULL) { return PR_TRUE; + } item = NSS_CMSContentInfo_GetContent(NSS_CMSMessage_GetContentInfo(cmsg)); From fa26771e9515cc82c941fcef689dd797a3e308c3 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Fri, 11 Jan 2019 22:33:16 -0700 Subject: [PATCH] Bug 1507174 - Add additional null checks to other CMS functions r=mt Differential review: https://phabricator.services.mozilla.com//D16383 --HG-- branch : NSS_3_36_BRANCH extra : transplant_source : %B5%A8su%96%5B%BE%F9%CD%93%E0%EE%93a4c%1BYp%09 --- lib/smime/cmscinfo.c | 92 ++++++++++++++++++++++++++++++++++++------ lib/smime/cmsdigdata.c | 4 +- lib/smime/cmsencdata.c | 4 +- lib/smime/cmsenvdata.c | 5 +++ lib/smime/cmsmessage.c | 3 ++ lib/smime/cmsudf.c | 2 +- 6 files changed, 95 insertions(+), 15 deletions(-) diff --git a/lib/smime/cmscinfo.c b/lib/smime/cmscinfo.c index 08db662f8..453ccaada 100644 --- a/lib/smime/cmscinfo.c +++ b/lib/smime/cmscinfo.c @@ -51,6 +51,10 @@ NSS_CMSContentInfo_Destroy(NSSCMSContentInfo *cinfo) { SECOidTag kind; + if (cinfo == NULL) { + return; + } + kind = NSS_CMSContentInfo_GetContentTypeTag(cinfo); switch (kind) { case SEC_OID_PKCS7_ENVELOPED_DATA: @@ -86,6 +90,11 @@ NSSCMSContentInfo * NSS_CMSContentInfo_GetChildContentInfo(NSSCMSContentInfo *cinfo) { NSSCMSContentInfo *ccinfo = NULL; + + if (cinfo == NULL) { + return NULL; + } + SECOidTag tag = NSS_CMSContentInfo_GetContentTypeTag(cinfo); switch (tag) { case SEC_OID_PKCS7_SIGNED_DATA: @@ -127,6 +136,9 @@ SECStatus NSS_CMSContentInfo_SetDontStream(NSSCMSContentInfo *cinfo, PRBool dontStream) { SECStatus rv; + if (cinfo == NULL) { + return SECFailure; + } rv = NSS_CMSContentInfo_Private_Init(cinfo); if (rv != SECSuccess) { @@ -145,15 +157,20 @@ NSS_CMSContentInfo_SetContent(NSSCMSMessage *cmsg, NSSCMSContentInfo *cinfo, SECOidTag type, void *ptr) { SECStatus rv; + if (cinfo == NULL || cmsg == NULL) { + return SECFailure; + } cinfo->contentTypeTag = SECOID_FindOIDByTag(type); - if (cinfo->contentTypeTag == NULL) + if (cinfo->contentTypeTag == NULL) { return SECFailure; + } /* do not copy the oid, just create a reference */ rv = SECITEM_CopyItem(cmsg->poolp, &(cinfo->contentType), &(cinfo->contentTypeTag->oid)); - if (rv != SECSuccess) + if (rv != SECSuccess) { return SECFailure; + } cinfo->content.pointer = ptr; @@ -185,8 +202,9 @@ SECStatus NSS_CMSContentInfo_SetContent_Data(NSSCMSMessage *cmsg, NSSCMSContentInfo *cinfo, SECItem *data, PRBool detached) { - if (NSS_CMSContentInfo_SetContent(cmsg, cinfo, SEC_OID_PKCS7_DATA, (void *)data) != SECSuccess) + if (NSS_CMSContentInfo_SetContent(cmsg, cinfo, SEC_OID_PKCS7_DATA, (void *)data) != SECSuccess) { return SECFailure; + } if (detached) { cinfo->rawContent = NULL; } @@ -230,6 +248,10 @@ NSS_CMSContentInfo_SetContent_EncryptedData(NSSCMSMessage *cmsg, NSSCMSContentIn void * NSS_CMSContentInfo_GetContent(NSSCMSContentInfo *cinfo) { + if (cinfo == NULL) { + return NULL; + } + SECOidTag tag = cinfo->contentTypeTag ? cinfo->contentTypeTag->offset : SEC_OID_UNKNOWN; @@ -260,6 +282,10 @@ NSS_CMSContentInfo_GetInnerContent(NSSCMSContentInfo *cinfo) SECOidTag tag; SECItem *pItem = NULL; + if (cinfo == NULL) { + return NULL; + } + tag = NSS_CMSContentInfo_GetContentTypeTag(cinfo); if (NSS_CMSType_IsData(tag)) { pItem = cinfo->content.data; @@ -282,6 +308,10 @@ NSS_CMSContentInfo_GetInnerContent(NSSCMSContentInfo *cinfo) SECOidTag NSS_CMSContentInfo_GetContentTypeTag(NSSCMSContentInfo *cinfo) { + if (cinfo == NULL) { + return SEC_OID_UNKNOWN; + } + if (cinfo->contentTypeTag == NULL) cinfo->contentTypeTag = SECOID_FindOID(&(cinfo->contentType)); @@ -294,11 +324,17 @@ NSS_CMSContentInfo_GetContentTypeTag(NSSCMSContentInfo *cinfo) SECItem * NSS_CMSContentInfo_GetContentTypeOID(NSSCMSContentInfo *cinfo) { - if (cinfo->contentTypeTag == NULL) + if (cinfo == NULL) { + return NULL; + } + + if (cinfo->contentTypeTag == NULL) { cinfo->contentTypeTag = SECOID_FindOID(&(cinfo->contentType)); + } - if (cinfo->contentTypeTag == NULL) + if (cinfo->contentTypeTag == NULL) { return NULL; + } return &(cinfo->contentTypeTag->oid); } @@ -310,8 +346,13 @@ NSS_CMSContentInfo_GetContentTypeOID(NSSCMSContentInfo *cinfo) SECOidTag NSS_CMSContentInfo_GetContentEncAlgTag(NSSCMSContentInfo *cinfo) { - if (cinfo->contentEncAlgTag == SEC_OID_UNKNOWN) + if (cinfo == NULL) { + return SEC_OID_UNKNOWN; + } + + if (cinfo->contentEncAlgTag == SEC_OID_UNKNOWN) { cinfo->contentEncAlgTag = SECOID_GetAlgorithmTag(&(cinfo->contentEncAlg)); + } return cinfo->contentEncAlgTag; } @@ -322,6 +363,10 @@ NSS_CMSContentInfo_GetContentEncAlgTag(NSSCMSContentInfo *cinfo) SECAlgorithmID * NSS_CMSContentInfo_GetContentEncAlg(NSSCMSContentInfo *cinfo) { + if (cinfo == NULL) { + return NULL; + } + return &(cinfo->contentEncAlg); } @@ -330,10 +375,14 @@ NSS_CMSContentInfo_SetContentEncAlg(PLArenaPool *poolp, NSSCMSContentInfo *cinfo SECOidTag bulkalgtag, SECItem *parameters, int keysize) { SECStatus rv; + if (cinfo == NULL) { + return SECFailure; + } rv = SECOID_SetAlgorithmID(poolp, &(cinfo->contentEncAlg), bulkalgtag, parameters); - if (rv != SECSuccess) + if (rv != SECSuccess) { return SECFailure; + } cinfo->keysize = keysize; return SECSuccess; } @@ -343,27 +392,42 @@ NSS_CMSContentInfo_SetContentEncAlgID(PLArenaPool *poolp, NSSCMSContentInfo *cin SECAlgorithmID *algid, int keysize) { SECStatus rv; + if (cinfo == NULL) { + return SECFailure; + } rv = SECOID_CopyAlgorithmID(poolp, &(cinfo->contentEncAlg), algid); - if (rv != SECSuccess) + if (rv != SECSuccess) { return SECFailure; - if (keysize >= 0) + } + if (keysize >= 0) { cinfo->keysize = keysize; + } return SECSuccess; } void NSS_CMSContentInfo_SetBulkKey(NSSCMSContentInfo *cinfo, PK11SymKey *bulkkey) { - cinfo->bulkkey = PK11_ReferenceSymKey(bulkkey); - cinfo->keysize = PK11_GetKeyStrength(cinfo->bulkkey, &(cinfo->contentEncAlg)); + if (cinfo == NULL) { + return; + } + + if (bulkkey == NULL) { + cinfo->bulkkey = NULL; + cinfo->keysize = 0; + } else { + cinfo->bulkkey = PK11_ReferenceSymKey(bulkkey); + cinfo->keysize = PK11_GetKeyStrength(cinfo->bulkkey, &(cinfo->contentEncAlg)); + } } PK11SymKey * NSS_CMSContentInfo_GetBulkKey(NSSCMSContentInfo *cinfo) { - if (cinfo->bulkkey == NULL) + if (cinfo == NULL || cinfo->bulkkey == NULL) { return NULL; + } return PK11_ReferenceSymKey(cinfo->bulkkey); } @@ -371,5 +435,9 @@ NSS_CMSContentInfo_GetBulkKey(NSSCMSContentInfo *cinfo) int NSS_CMSContentInfo_GetBulkKeySize(NSSCMSContentInfo *cinfo) { + if (cinfo == NULL) { + return 0; + } + return cinfo->keysize; } diff --git a/lib/smime/cmsdigdata.c b/lib/smime/cmsdigdata.c index 9ea22702e..a249686bb 100644 --- a/lib/smime/cmsdigdata.c +++ b/lib/smime/cmsdigdata.c @@ -56,7 +56,9 @@ void NSS_CMSDigestedData_Destroy(NSSCMSDigestedData *digd) { /* everything's in a pool, so don't worry about the storage */ - NSS_CMSContentInfo_Destroy(&(digd->contentInfo)); + if (digd != NULL) { + NSS_CMSContentInfo_Destroy(&(digd->contentInfo)); + } return; } diff --git a/lib/smime/cmsencdata.c b/lib/smime/cmsencdata.c index c3a4549ad..8b520b439 100644 --- a/lib/smime/cmsencdata.c +++ b/lib/smime/cmsencdata.c @@ -87,7 +87,9 @@ void NSS_CMSEncryptedData_Destroy(NSSCMSEncryptedData *encd) { /* everything's in a pool, so don't worry about the storage */ - NSS_CMSContentInfo_Destroy(&(encd->contentInfo)); + if (encd != NULL) { + NSS_CMSContentInfo_Destroy(&(encd->contentInfo)); + } return; } diff --git a/lib/smime/cmsenvdata.c b/lib/smime/cmsenvdata.c index f2c8e171d..9bc77be8b 100644 --- a/lib/smime/cmsenvdata.c +++ b/lib/smime/cmsenvdata.c @@ -144,6 +144,11 @@ NSS_CMSEnvelopedData_Encode_BeforeStart(NSSCMSEnvelopedData *envd) poolp = envd->cmsg->poolp; cinfo = &(envd->contentInfo); + if (cinfo == NULL) { + PORT_SetError(SEC_ERROR_BAD_DATA); + goto loser; + } + recipientinfos = envd->recipientInfos; if (recipientinfos == NULL) { PORT_SetError(SEC_ERROR_BAD_DATA); diff --git a/lib/smime/cmsmessage.c b/lib/smime/cmsmessage.c index f41a432b1..366b71aba 100644 --- a/lib/smime/cmsmessage.c +++ b/lib/smime/cmsmessage.c @@ -96,6 +96,9 @@ NSS_CMSMessage_SetEncodingParams(NSSCMSMessage *cmsg, void NSS_CMSMessage_Destroy(NSSCMSMessage *cmsg) { + if (cmsg == NULL) + return; + PORT_Assert(cmsg->refCount > 0); if (cmsg->refCount <= 0) { /* oops */ return; diff --git a/lib/smime/cmsudf.c b/lib/smime/cmsudf.c index 3ef4268d4..5c8a81e6d 100644 --- a/lib/smime/cmsudf.c +++ b/lib/smime/cmsudf.c @@ -239,7 +239,7 @@ NSS_CMSGenericWrapperData_Destroy(SECOidTag type, NSSCMSGenericWrapperData *gd) { const nsscmstypeInfo *typeInfo = nss_cmstype_lookup(type); - if (typeInfo && typeInfo->destroy) { + if (typeInfo && (typeInfo->destroy) && (gd != NULL)) { (*typeInfo->destroy)(gd); } }