Bug 1507135 - Add additional null checks to CMS message functions r=mt
authorJ.C. Jones <jjones@mozilla.com>
Mon, 14 Jan 2019 10:35:25 -0700
changeset 14965 5e70b72131ac28457b14cdc6100e8674409bbdd4
parent 14964 da45424cb9a0b4d8e45e5040e2e3b574d994e254
child 14966 08d1b0c1117f7a9a5382440864e243ece0d1a7a3
push id3241
push userjjones@mozilla.com
push dateSun, 20 Jan 2019 00:41:34 +0000
reviewersmt
bugs1507135
Bug 1507135 - Add additional null checks to CMS message functions r=mt Differential review: https://phabricator.services.mozilla.com//D16488
lib/smime/cmsmessage.c
--- a/lib/smime/cmsmessage.c
+++ b/lib/smime/cmsmessage.c
@@ -24,42 +24,45 @@ NSSCMSMessage *
 NSS_CMSMessage_Create(PLArenaPool *poolp)
 {
     void *mark = NULL;
     NSSCMSMessage *cmsg;
     PRBool poolp_is_ours = PR_FALSE;
 
     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;
     }
 
     if (!poolp_is_ours)
         mark = PORT_ArenaMark(poolp);
 
     cmsg = (NSSCMSMessage *)PORT_ArenaZAlloc(poolp, sizeof(NSSCMSMessage));
     if (cmsg == NULL ||
         NSS_CMSContentInfo_Private_Init(&(cmsg->contentInfo)) != SECSuccess) {
         if (!poolp_is_ours) {
             if (mark) {
                 PORT_ArenaRelease(poolp, mark);
             }
-        } else
+        } else {
             PORT_FreeArena(poolp, PR_FALSE);
+        }
         return NULL;
     }
 
     cmsg->poolp = poolp;
     cmsg->poolp_is_ours = poolp_is_ours;
     cmsg->refCount = 1;
 
-    if (mark)
+    if (mark) {
         PORT_ArenaUnmark(poolp, mark);
+    }
 
     return cmsg;
 }
 
 /*
  * NSS_CMSMessage_SetEncodingParams - set up a CMS message object for encoding or decoding
  *
  * "cmsg" - message object
@@ -68,90 +71,111 @@ NSS_CMSMessage_Create(PLArenaPool *poolp
  * "detached_digestalgs", "detached_digests" - digests from detached content
  */
 void
 NSS_CMSMessage_SetEncodingParams(NSSCMSMessage *cmsg,
                                  PK11PasswordFunc pwfn, void *pwfn_arg,
                                  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;
     cmsg->detached_digestalgs = detached_digestalgs;
     cmsg->detached_digests = detached_digests;
 }
 
 /*
  * NSS_CMSMessage_Destroy - destroy a CMS message and all of its sub-pieces.
  */
 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? */
+    }
 }
 
 /*
  * NSS_CMSMessage_Copy - return a copy of the given message.
  *
  * The copy may be virtual or may be real -- either way, the result needs
  * to be passed to NSS_CMSMessage_Destroy later (as does the original).
  */
 NSSCMSMessage *
 NSS_CMSMessage_Copy(NSSCMSMessage *cmsg)
 {
-    if (cmsg == NULL)
+    if (cmsg == NULL) {
         return NULL;
+    }
 
     PORT_Assert(cmsg->refCount > 0);
 
     cmsg->refCount++; /* XXX chrisk thread safety? */
     return cmsg;
 }
 
 /*
  * NSS_CMSMessage_GetArena - return a pointer to the message's arena pool
  */
 PLArenaPool *
 NSS_CMSMessage_GetArena(NSSCMSMessage *cmsg)
 {
+    if (cmsg == NULL) {
+        return NULL;
+    }
+
     return cmsg->poolp;
 }
 
 /*
  * NSS_CMSMessage_GetContentInfo - return a pointer to the top level contentInfo
  */
 NSSCMSContentInfo *
 NSS_CMSMessage_GetContentInfo(NSSCMSMessage *cmsg)
 {
+    if (cmsg == NULL) {
+        return NULL;
+    }
+
     return &(cmsg->contentInfo);
 }
 
 /*
  * Return a pointer to the actual content.
  * In the case of those types which are encrypted, this returns the *plain* content.
  * In case of nested contentInfos, this descends and retrieves the innermost content.
  */
 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);
     return pItem;
 }
 
 /*
  * NSS_CMSMessage_ContentLevelCount - count number of levels of CMS content objects in this message
@@ -159,16 +183,20 @@ NSS_CMSMessage_GetContent(NSSCMSMessage 
  * CMS data content objects do not count.
  */
 int
 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++;
         cinfo = NSS_CMSContentInfo_GetChildContentInfo(cinfo);
     }
     return count;
 }
 
@@ -178,16 +206,20 @@ NSS_CMSMessage_ContentLevelCount(NSSCMSM
  * CMS data content objects do not count.
  */
 NSSCMSContentInfo *
 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)) {
         count++;
     }
 
     return cinfo;
 }
@@ -195,16 +227,20 @@ NSS_CMSMessage_ContentLevel(NSSCMSMessag
 /*
  * NSS_CMSMessage_ContainsCertsOrCrls - see if message contains certs along the way
  */
 PRBool
 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)) {
         if (!NSS_CMSType_IsData(NSS_CMSContentInfo_GetContentTypeTag(cinfo)))
             continue; /* next level */
 
         if (NSS_CMSSignedData_ContainsCertsOrCrls(cinfo->content.signedData))
             return PR_TRUE;
@@ -216,16 +252,20 @@ NSS_CMSMessage_ContainsCertsOrCrls(NSSCM
 /*
  * NSS_CMSMessage_IsEncrypted - see if message contains a encrypted submessage
  */
 PRBool
 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)) {
         switch (NSS_CMSContentInfo_GetContentTypeTag(cinfo)) {
             case SEC_OID_PKCS7_ENVELOPED_DATA:
             case SEC_OID_PKCS7_ENCRYPTED_DATA:
                 return PR_TRUE;
             default:
@@ -246,23 +286,31 @@ NSS_CMSMessage_IsEncrypted(NSSCMSMessage
  * Note that the content itself can be empty (detached content was sent
  * another way); it is the presence of the signature that matters.
  */
 PRBool
 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? */
                 break;
         }
     }
     return PR_FALSE;
 }
@@ -273,18 +321,19 @@ NSS_CMSMessage_IsSigned(NSSCMSMessage *c
  * returns PR_TRUE is innermost content length is < minLen
  * XXX need the encrypted content length (why?)
  */
 PRBool
 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));
 
     if (!item) {
         return PR_TRUE;
     } else if (item->len <= minLen) {
         return PR_TRUE;
     }