Bug 1277228 - some scan-build fixes to enable it on base/certdb/certhigh, r=ttaubert
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Mon, 17 Oct 2016 16:49:34 +0200
changeset 12734 25bbb8bdd46086979558331d5030adeab4d4129e
parent 12733 9504c6de8b80a1cb8a14fbdb4bf7c2fe751e047f
child 12735 7ddb150d29c160cf6045d07e9ea56aed50940923
push id1679
push userfranziskuskiefer@gmail.com
push dateTue, 18 Oct 2016 17:42:14 +0000
reviewersttaubert
bugs1277228
Bug 1277228 - some scan-build fixes to enable it on base/certdb/certhigh, r=ttaubert try: -t all
automation/taskcluster/scripts/run_scan_build.sh
build.sh
cmd/ecperf/ecperf.c
lib/base/list.c
lib/certdb/certdb.c
lib/certdb/certt.h
lib/certdb/crl.c
lib/certdb/genname.c
lib/certdb/secname.c
lib/certdb/stanpcertdb.c
lib/certhigh/certhigh.c
lib/certhigh/ocsp.c
--- a/automation/taskcluster/scripts/run_scan_build.sh
+++ b/automation/taskcluster/scripts/run_scan_build.sh
@@ -15,28 +15,31 @@ fi
 # Build.
 cd nss
 make nss_build_all
 
 # What we want to scan.
 # key: directory to scan
 # value: number of errors expected in that directory
 declare -A scan=( \
+        [lib/base]=0 \
+        [lib/certdb]=0 \
+        [lib/certhigh]=0 \
         [lib/ssl]=0 \
         [lib/freebl]=0 \
         [lib/util]=0 \
     )
 
 # remove .OBJ directories to force a rebuild of just the select few
 for i in "${!scan[@]}"; do
    find "$i" -name "*.OBJ" -exec rm -rf {} \+
 done
 
 # run scan-build (only building affected directories)
-scan-build -o /home/worker/artifacts --use-cc=$(CC) --use-c++=$(CCC) make nss_build_all && cd ..
+scan-build -o /home/worker/artifacts --use-cc=$CC --use-c++=$CCC make nss_build_all && cd ..
 
 # print errors we found
 set +v +x
 STATUS=0
 for i in "${!scan[@]}"; do
    n=$(grep -Rn "$i" /home/worker/artifacts/*/report-*.html | wc -l)
    if [ $n -ne ${scan[$i]} ]; then
      STATUS=1
--- a/build.sh
+++ b/build.sh
@@ -8,21 +8,21 @@ DIST_DIR="$CWD/../dist/$OBJ_DIR"
 NSS_GYP=1 make install_nspr
 
 if [ -z "${USE_64}" ]; then
     GYP_PARAMS="-Dtarget_arch=ia32"
 fi
 
 # generate NSS build files only if asked for it
 if [ -n "${NSS_GYP_GEN}" -o ! -d out/Debug ]; then
-    PKG_CONFIG_PATH="$CWD/../nspr/$OBJ_DIR/config" gyp -f ninja $GYP_PARAMS --depth=. nss.gyp
+    PKG_CONFIG_PATH="$CWD/../nspr/$OBJ_DIR/config" $SCANBUILD gyp -f ninja $GYP_PARAMS --depth=. nss.gyp
 fi
 # build NSS
 # TODO: only doing this for debug build for now
-ninja -C out/Debug/
+$SCANBUILD ninja -C out/Debug/
 if [ $? != 0 ]; then
     exit 1
 fi
 
 # sign libs
 # TODO: this is done every time at the moment.
 cd out/Debug/
 LD_LIBRARY_PATH=$DIST_DIR/lib/ ./shlibsign -v -i lib/libfreebl3.so
--- a/cmd/ecperf/ecperf.c
+++ b/cmd/ecperf/ecperf.c
@@ -465,17 +465,17 @@ ectest_curve_pkcs11(ECCurveName curve, i
     SECItem digest;
     SECKEYECParams ecParams;
     CK_MECHANISM mech;
     CK_ECDH1_DERIVE_PARAMS ecdh_params;
     unsigned char sigData[256];
     unsigned char digestData[20];
     unsigned char pubKeyData[256];
     PRLock *lock = NULL;
-    double signRate, deriveRate;
+    double signRate, deriveRate = 0;
     CK_ATTRIBUTE template;
     SECStatus rv;
     CK_RV crv;
 
     ecParams.data = NULL;
     ecParams.len = 0;
     rv = ecName2params(curve, &ecParams);
     if (rv != SECSuccess) {
@@ -590,17 +590,17 @@ ectest_curve_freebl(ECCurveName curve, i
 {
     ECParams ecParams = { 0 };
     ECPrivateKey *ecPriv = NULL;
     ECPublicKey ecPub;
     SECItem sig;
     SECItem digest;
     unsigned char sigData[256];
     unsigned char digestData[20];
-    double signRate, deriveRate;
+    double signRate, deriveRate = 0;
     char genenc[3 + 2 * 2 * MAX_ECKEY_LEN];
     SECStatus rv = SECFailure;
     PLArenaPool *arena;
 
     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
     if (!arena) {
         return SECFailure;
     }
--- a/lib/base/list.c
+++ b/lib/base/list.c
@@ -47,30 +47,27 @@ static PRBool
 pointer_compare(void *a, void *b)
 {
     return (PRBool)(a == b);
 }
 
 static nssListElement *
 nsslist_get_matching_element(nssList *list, void *data)
 {
-    PRCList *link;
     nssListElement *node;
     node = list->head;
     if (!node) {
         return NULL;
     }
-    link = &node->link;
     while (node) {
         /* using a callback slows things down when it's just compare ... */
         if (list->compareFunc(node->data, data)) {
             break;
         }
-        link = &node->link;
-        if (link == PR_LIST_TAIL(&list->head->link)) {
+        if (&node->link == PR_LIST_TAIL(&list->head->link)) {
             node = NULL;
             break;
         }
         node = (nssListElement *)PR_NEXT_LINK(&node->link);
     }
     return node;
 }
 
--- a/lib/certdb/certdb.c
+++ b/lib/certdb/certdb.c
@@ -1290,22 +1290,26 @@ CERT_AddOKDomainName(CERTCertificate *ce
 {
     CERTOKDomainName *domainOK;
     int newNameLen;
 
     if (!hn || !(newNameLen = strlen(hn))) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
-    domainOK = (CERTOKDomainName *)PORT_ArenaZAlloc(
-        cert->arena, (sizeof *domainOK) + newNameLen);
-    if (!domainOK)
+    domainOK = (CERTOKDomainName *)PORT_ArenaZAlloc(cert->arena, sizeof(*domainOK));
+    if (!domainOK) {
         return SECFailure; /* error code is already set. */
-
-    PORT_Strcpy(domainOK->name, hn);
+    }
+    domainOK->name = (char *)PORT_ArenaZAlloc(cert->arena, newNameLen + 1);
+    if (!domainOK->name) {
+        return SECFailure; /* error code is already set. */
+    }
+
+    PORT_Strncpy(domainOK->name, hn, newNameLen + 1);
     sec_lower_string(domainOK->name);
 
     /* put at head of list. */
     domainOK->next = cert->domainOK;
     cert->domainOK = domainOK;
     return SECSuccess;
 }
 
@@ -1397,17 +1401,16 @@ cert_VerifySubjectAltName(const CERTCert
     cnBufLen = sizeof cnbuf;
 
     rv = CERT_FindCertExtension(cert, SEC_OID_X509_SUBJECT_ALT_NAME,
                                 &subAltName);
     if (rv != SECSuccess) {
         goto fail;
     }
     isIPaddr = (PR_SUCCESS == PR_StringToNetAddr(hn, &netAddr));
-    rv = SECFailure;
     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
     if (!arena)
         goto fail;
 
     nameList = current = CERT_DecodeAltNameExtension(arena, &subAltName);
     if (!current)
         goto fail;
 
--- a/lib/certdb/certt.h
+++ b/lib/certdb/certt.h
@@ -728,17 +728,17 @@ struct CERTVerifyLogStr {
     PLArenaPool *arena;
     unsigned int count;
     struct CERTVerifyLogNodeStr *head;
     struct CERTVerifyLogNodeStr *tail;
 };
 
 struct CERTOKDomainNameStr {
     CERTOKDomainName *next;
-    char name[1]; /* actual length may be longer. */
+    char *name;
 };
 
 typedef SECStatus(PR_CALLBACK *CERTStatusChecker)(CERTCertDBHandle *handle,
                                                   CERTCertificate *cert,
                                                   PRTime time, void *pwArg);
 
 typedef SECStatus(PR_CALLBACK *CERTStatusDestroy)(CERTStatusConfig *handle);
 
--- a/lib/certdb/crl.c
+++ b/lib/certdb/crl.c
@@ -2777,17 +2777,17 @@ cert_CacheCRLByGeneralName(CERTCertDBHan
     PORT_Assert(SECSuccess == rv);
     if (SECSuccess != rv) {
         SECITEM_ZfreeItem(crl, PR_TRUE);
         return SECFailure;
     }
     rv = cert_FindCRLByGeneralName(ncc, canonicalizedName, &oldEntry);
     PORT_Assert(SECSuccess == rv);
     if (SECSuccess != rv) {
-        rv = cert_ReleaseNamedCRLCache(ncc);
+        (void)cert_ReleaseNamedCRLCache(ncc);
         SECITEM_ZfreeItem(crl, PR_TRUE);
         return SECFailure;
     }
     if (SECSuccess ==
         addCRLToCache(dbhandle, crl, canonicalizedName, &newEntry)) {
         if (!oldEntry) {
             /* add new good entry to the hash table */
             if (NULL == PL_HashTableAdd(namedCRLCache.entries,
--- a/lib/certdb/genname.c
+++ b/lib/certdb/genname.c
@@ -298,17 +298,17 @@ CERT_GetPrevNameConstraint(CERTNameConst
 SECItem *
 CERT_EncodeGeneralName(CERTGeneralName *genName, SECItem *dest,
                        PLArenaPool *arena)
 {
 
     const SEC_ASN1Template *template;
 
     PORT_Assert(arena);
-    if (arena == NULL) {
+    if (arena == NULL || !genName) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return NULL;
     }
     /* TODO: mark arena */
     if (dest == NULL) {
         dest = PORT_ArenaZNew(arena, SECItem);
         if (!dest)
             goto loser;
@@ -371,26 +371,27 @@ loser:
     return NULL;
 }
 
 SECItem **
 cert_EncodeGeneralNames(PLArenaPool *arena, CERTGeneralName *names)
 {
     CERTGeneralName *current_name;
     SECItem **items = NULL;
-    int count = 0;
+    int count = 1;
     int i;
     PRCList *head;
 
+    if (!names) {
+        return NULL;
+    }
+
     PORT_Assert(arena);
     /* TODO: mark arena */
     current_name = names;
-    if (names != NULL) {
-        count = 1;
-    }
     head = &(names->l);
     while (current_name->l.next != head) {
         current_name = CERT_GetNextGeneralName(current_name);
         ++count;
     }
     current_name = CERT_GetNextGeneralName(current_name);
     items = PORT_ArenaNewArray(arena, SECItem *, count + 1);
     if (items == NULL) {
@@ -1064,17 +1065,17 @@ cert_ExtractDNEmailAddrs(CERTGeneralName
                 SECITEM_FreeItem(avaValue, PR_TRUE);
                 if (rv != SECSuccess)
                     goto loser;
                 nameList = cert_CombineNamesLists(nameList, newName);
             } /* handle one email AVA */
         }     /* loop over AVAs */
     }         /* loop over RDNs */
     /* combine new names with old one. */
-    name = cert_CombineNamesLists(name, nameList);
+    (void)cert_CombineNamesLists(name, nameList);
     /* TODO: unmark arena */
     return SECSuccess;
 
 loser:
     /* TODO: release arena back to mark */
     return SECFailure;
 }
 
--- a/lib/certdb/secname.c
+++ b/lib/certdb/secname.c
@@ -583,18 +583,21 @@ CERT_CompareName(const CERTName *a, cons
     ac = CountArray((void **)ardns);
     bc = CountArray((void **)brdns);
     if (ac < bc)
         return SECLessThan;
     if (ac > bc)
         return SECGreaterThan;
 
     for (;;) {
-        ardn = *ardns++;
-        brdn = *brdns++;
+        if (!ardns++ || !brdns++) {
+            break;
+        }
+        ardn = *ardns;
+        brdn = *brdns;
         if (!ardn) {
             break;
         }
         rv = CERT_CompareRDN(ardn, brdn);
         if (rv)
             return rv;
     }
     return rv;
--- a/lib/certdb/stanpcertdb.c
+++ b/lib/certdb/stanpcertdb.c
@@ -153,18 +153,16 @@ extern const NSSError NSS_ERROR_PKCS11;
 void
 CERT_MapStanError()
 {
     PRInt32 *errorStack;
     NSSError error, prevError;
     int secError;
     int i;
 
-    error = 0;
-
     errorStack = NSS_GetErrorStack();
     if (errorStack == 0) {
         PORT_SetError(0);
         return;
     }
     error = prevError = CKR_GENERAL_ERROR;
     /* get the 'top 2' error codes from the stack */
     for (i = 0; errorStack[i]; i++) {
@@ -854,17 +852,17 @@ certdb_SaveSingleProfile(CERTCertificate
             }
         } else {
             saveit = PR_TRUE;
         }
     }
 
     if (saveit) {
         if (cc) {
-            if (stanProfile) {
+            if (stanProfile && profileTime && emailProfile) {
                 /* stanProfile is already stored in the crypto context,
                  * overwrite the data
                  */
                 NSSArena *arena = stanProfile->object.arena;
                 stanProfile->profileTime = nssItem_Create(
                     arena, NULL, profileTime->len, profileTime->data);
                 stanProfile->profileData = nssItem_Create(
                     arena, NULL, emailProfile->len, emailProfile->data);
--- a/lib/certhigh/certhigh.c
+++ b/lib/certhigh/certhigh.c
@@ -142,16 +142,20 @@ CERT_FindUserCertsByUsage(CERTCertDBHand
         nn = nicknames->numnicknames;
         nnptr = nicknames->nicknames;
 
         flags = (PRBool *)PORT_ZAlloc(sizeof(PRBool) * nn);
         if (flags == NULL) {
             goto loser;
         }
 
+        if (!certList) {
+            goto loser;
+        }
+
         node = CERT_LIST_HEAD(certList);
 
         /* treverse all certs in the list */
         while (!CERT_LIST_END(node, certList)) {
 
             /* find matching nickname index */
             for (n = 0; n < nn; n++) {
                 if (CERT_MatchNickname(nnptr[n], node->cert->nickname)) {
--- a/lib/certhigh/ocsp.c
+++ b/lib/certhigh/ocsp.c
@@ -4123,19 +4123,17 @@ CERT_VerifyOCSPResponseSignature(CERTOCS
     rv = DER_GeneralizedTimeToTime(&producedAt, &tbsData->producedAt);
     if (rv != SECSuccess)
         goto finish;
 
     /*
      * Just because we have a cert does not mean it is any good; check
      * it for validity, trust and usage.
      */
-    if (ocsp_CertIsOCSPDefaultResponder(handle, signerCert)) {
-        rv = SECSuccess;
-    } else {
+    if (!ocsp_CertIsOCSPDefaultResponder(handle, signerCert)) {
         SECCertUsage certUsage;
         if (CERT_IsCACert(signerCert, NULL)) {
             certUsage = certUsageAnyCA;
         } else {
             certUsage = certUsageStatusResponder;
         }
         rv = cert_VerifyCertWithFlags(handle, signerCert, PR_TRUE, certUsage,
                                       producedAt, CERT_VERIFYCERT_SKIP_OCSP,