Bug 1507135 - Add additional null checks to CMS message functions r=mt

Differential review: https://phabricator.services.mozilla.com//D16488

1 files 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));