Bug 1280844 - Fix up null checks in cmd/. r?mt. draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 21 Jun 2016 09:59:03 +1000
changeset 380168 1d074646eac0776e03818fec6ab2dede4f6aa9f3
parent 379915 50e8df81c47d8afde6a36009edc6d728d6228104
child 380169 861c92565f6c16310f84953804e9365d7c1302cc
push id21158
push usernnethercote@mozilla.com
push dateMon, 20 Jun 2016 23:59:39 +0000
reviewersmt
bugs1280844
milestone50.0a1
Bug 1280844 - Fix up null checks in cmd/. r?mt. This patch: - Adds some missing null checks after allocations. - Moves some derefs of pointers after null checks of those same pointers. - Removes some redundant null checks for pointers that are guaranteed to be non-null. MozReview-Commit-ID: FAva5Wg5fr
security/nss/cmd/certcgi/certcgi.c
security/nss/cmd/certutil/certext.c
security/nss/cmd/crlutil/crlgen.c
security/nss/cmd/lib/secutil.c
security/nss/cmd/pk11mode/pk11mode.c
security/nss/cmd/pk12util/pk12util.c
--- a/security/nss/cmd/certcgi/certcgi.c
+++ b/security/nss/cmd/certcgi/certcgi.c
@@ -914,17 +914,23 @@ AddPrivKeyUsagePeriod(void *extHandle,
     if (!arena) {
         error_allocate();
     }
     pkup = PORT_ArenaZNew(arena, CERTPrivKeyUsagePeriod);
     if (pkup == NULL) {
         error_allocate();
     }
     notBeforeStr = (char *)PORT_Alloc(16);
+    if (notBeforeStr == NULL) {
+        error_allocate();
+    }
     notAfterStr = (char *)PORT_Alloc(16);
+    if (notAfterStr == NULL) {
+        error_allocate();
+    }
     *notBeforeStr = '\0';
     *notAfterStr = '\0';
     pkup->arena = arena;
     pkup->notBefore.len = 0;
     pkup->notBefore.data = NULL;
     pkup->notAfter.len = 0;
     pkup->notAfter.data = NULL;
     if (find_field_bool(data, "privKeyUsagePeriod-radio-notBefore", PR_TRUE) ||
@@ -1028,25 +1034,19 @@ AddPrivKeyUsagePeriod(void *extHandle,
 
     rv = EncodeAndAddExtensionValue(arena, extHandle, pkup,
                                     find_field_bool(data,
                                                     "privKeyUsagePeriod-crit",
                                                     PR_TRUE),
                                     SEC_OID_X509_PRIVATE_KEY_USAGE_PERIOD,
                                     (EXTEN_VALUE_ENCODER)
                                         CERT_EncodePrivateKeyUsagePeriod);
-    if (arena) {
-        PORT_FreeArena(arena, PR_FALSE);
-    }
-    if (notBeforeStr != NULL) {
-        PORT_Free(notBeforeStr);
-    }
-    if (notAfterStr != NULL) {
-        PORT_Free(notAfterStr);
-    }
+    PORT_FreeArena(arena, PR_FALSE);
+    PORT_Free(notBeforeStr);
+    PORT_Free(notAfterStr);
     return (rv);
 }
 
 static SECStatus
 AddBasicConstraint(void *extHandle,
                    Pair *data)
 {
     CERTBasicConstraints basicConstraint;
--- a/security/nss/cmd/certutil/certext.c
+++ b/security/nss/cmd/certutil/certext.c
@@ -267,24 +267,25 @@ GetYesNo(char *prompt)
  * nextPos is set to the token after found comma separator or to NULL.
  * NULL in nextPos should be used as indication of the last parsed token.
  * A special value "critical" can be parsed out from the supplied sting.*/
 
 static SECStatus
 parseNextCmdInput(const char *const *valueArray, int *value, char **nextPos,
                   PRBool *critical)
 {
-    char *thisPos = *nextPos;
+    char *thisPos;
     int keyLen = 0;
     int arrIndex = 0;
 
     if (!valueArray || !value || !nextPos || !critical) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
+    thisPos = *nextPos;
     while (1) {
         if ((*nextPos = strchr(thisPos, ',')) == NULL) {
             keyLen = strlen(thisPos);
         } else {
             keyLen = *nextPos - thisPos;
             *nextPos += 1;
         }
         /* if critical keyword is found, go for another loop,
--- a/security/nss/cmd/crlutil/crlgen.c
+++ b/security/nss/cmd/crlutil/crlgen.c
@@ -478,17 +478,16 @@ loser:
 /* Creates and adds CRLNumber extension to extension handle.
  * Since, this is CRL extension, extension handle is the one
  * related to CRL extensions */
 static SECStatus
 crlgen_AddCrlNumber(CRLGENGeneratorData *crlGenData, const char **dataArr)
 {
     PLArenaPool *arena = NULL;
     SECItem encodedItem;
-    void *extHandle = crlGenData->crlExtHandle;
     void *dummy;
     SECStatus rv = SECFailure;
     int code = 0;
 
     PORT_Assert(dataArr && crlGenData);
     if (!crlGenData || !dataArr) {
         goto loser;
     }
@@ -512,17 +511,18 @@ crlgen_AddCrlNumber(CRLGENGeneratorData 
     }
 
     dummy = SEC_ASN1EncodeInteger(arena, &encodedItem, code);
     if (!dummy) {
         rv = SECFailure;
         goto loser;
     }
 
-    rv = CERT_AddExtension(extHandle, SEC_OID_X509_CRL_NUMBER, &encodedItem,
+    rv = CERT_AddExtension(crlGenData->crlExtHandle, SEC_OID_X509_CRL_NUMBER,
+                           &encodedItem,
                            (*dataArr[1] == '1') ? PR_TRUE : PR_FALSE,
                            PR_TRUE);
 
 loser:
     if (arena)
         PORT_FreeArena(arena, PR_FALSE);
     return rv;
 }
--- a/security/nss/cmd/lib/secutil.c
+++ b/security/nss/cmd/lib/secutil.c
@@ -3508,19 +3508,17 @@ SECU_SignAndEncodeCRL(CERTCertificate *i
                                SEC_ASN1_GET(CERT_SignedCrlTemplate));
     if (!dummy) {
         *resCode = failToEncode;
         rv = SECFailure;
         goto done;
     }
 
 done:
-    if (caPrivateKey) {
-        SECKEY_DestroyPrivateKey(caPrivateKey);
-    }
+    SECKEY_DestroyPrivateKey(caPrivateKey);
     return rv;
 }
 
 SECStatus
 SECU_CopyCRL(PLArenaPool *destArena, CERTCrl *destCrl, CERTCrl *srcCrl)
 {
     void *dummy;
     SECStatus rv = SECSuccess;
--- a/security/nss/cmd/pk11mode/pk11mode.c
+++ b/security/nss/cmd/pk11mode/pk11mode.c
@@ -686,18 +686,17 @@ main(int argc, char **argv)
     if (doForkTests) {
         /* try to fork with softoken still loaded, but de-initialized */
         crv = PKM_ForkCheck(CKR_CRYPTOKI_NOT_INITIALIZED, pFunctionList,
                             PR_TRUE, NULL);
         if (crv != CKR_OK)
             goto cleanup;
     }
 
-    if (pSlotList)
-        free(pSlotList);
+    free(pSlotList);
 
     /* demonstrate how an application can be in Hybrid mode */
     /* PKM_HybridMode shows how to switch between NONFIPS */
     /* mode to FIPS mode */
 
     PKM_LogIt("Testing Hybrid mode \n");
     crv = PKM_HybridMode(pwd, pwdLen, &initArgs);
     if (crv == CKR_OK) {
@@ -1892,18 +1891,17 @@ PKM_ShowInfo(CK_FUNCTION_LIST_PTR pFunct
               (int)tokenInfo.hardwareVersion.minor);
     PKM_LogIt("    firmware version number: %d.%d\n",
               (int)tokenInfo.firmwareVersion.major,
               (int)tokenInfo.firmwareVersion.minor);
     if (tokenInfo.flags & CKF_CLOCK_ON_TOKEN) {
         PKM_LogIt("    current time: %.16s\n", tokenInfo.utcTime);
     }
     PKM_LogIt("PKM_ShowInfo done \n\n");
-    if (pSlotList)
-        free(pSlotList);
+    free(pSlotList);
     return crv;
 }
 
 /* PKM_HybridMode                                                         */
 /* The NSS cryptographic module has two modes of operation: FIPS Approved */
 /* mode and NONFIPS Approved mode. The two modes of operation are         */
 /* independent of each other -- they have their own copies of data        */
 /* structures and they are even allowed to be active at the same time.    */
--- a/security/nss/cmd/pk12util/pk12util.c
+++ b/security/nss/cmd/pk12util/pk12util.c
@@ -206,30 +206,32 @@ p12u_ucs2_ascii_conversion_function(PRBo
             /*if (i%60 == 0) printf("\n");*/
         }
         printf("\n");
     }
 #endif
     it.data = inBuf;
     it.len = inBufLen;
     dup = SECITEM_DupItem(&it);
+    if (!dup) {
+        return PR_FALSE;
+    }
     /* If converting Unicode to ASCII, swap bytes before conversion
      * as neccessary.
      */
     if (!toUnicode && swapBytes) {
         if (p12u_SwapUnicodeBytes(dup) != SECSuccess) {
             SECITEM_ZfreeItem(dup, PR_TRUE);
             return PR_FALSE;
         }
     }
     /* Perform the conversion. */
     ret = PORT_UCS2_UTF8Conversion(toUnicode, dup->data, dup->len,
                                    outBuf, maxOutBufLen, outBufLen);
-    if (dup)
-        SECITEM_ZfreeItem(dup, PR_TRUE);
+    SECITEM_ZfreeItem(dup, PR_TRUE);
 
 #ifdef DEBUG_CONVERSION
     if (pk12_debugging) {
         int i;
         printf("Converted to:\n");
         for (i = 0; i < *outBufLen; i++) {
             printf("%2x ", outBuf[i]);
             /*if (i%60 == 0) printf("\n");*/