Bug 1703936 New coverity/cpp scanner errors.

Redhat has run our scanners on the full NSS tree and identifed 123
errors our security team determined where 'critical'. I've reviewed
the errors and identified a much smaller subset of errors that are
either real, or confusing enough to warrent suppression comments.

Many errors are in cmd and gtest. I've skip those commands red hat does
not ship in this report, and I've skipped the issues in gtest.

Also, There's a large number of leaked_storage errors because evidently
coverity gets confused when you have a pointer in a local variable and
you pass that pointer off to a global or a function variable. I've
skipped most of those as well.

changes:
crlutil.c: add missing arena free in error path. #def4
secutil.c: (Not coverity found) make sure we don't overflow our buffer
           in badly encoded ECC oids.
modultil.c: Fix incorrect free operation in pk11install case. #def6
signtool/javascript.c: free old archiveDir after use in PR_smprintf #def8
                       don't double free curitem (curitem is almost
                       certainly NULL at this point, so the current code
                       is a noop). #def9, #def10
signtool/list.c: remove unused ugly_list variable (which is leaked) #def12
signtool/util.c: don't leak 'dir' in error path #def13 #def14
sigver/pk7print.c: coverity: use static pk rather than an allocated and
                   leaked pointer. #def15
                   add code for EC
                   disbled DSA code that isn't actually working
                   (PQG params).
symkeyutil.c: free name (depends on PORT_Free null check). #def16
pkix_pl_nameconstraints.c: Fix coverity double free warning.
                           PKIX_ERROR_RECEIVED is almost
                           certainly false in this case, but by
                           setting arena to NULL we make
                           sure it's not used or freed again. #def99
pkix_pl_string.c: Fix varargs leak in the error path. #def109
pk11parse.c: secmod_doDescCopy can reallocate our newSpec, but the
             pointer we are passed is an offset from newSpec. Pass in
             both pointers and return our newly allocated spec and
             length in that case.#def113
cmsutil.c: suppress cppcheck warnings do to cmsutil use of unions to
           cast pointers. #def113-117
pkcs11.c: support coverity incorrect use_after_free warning. #def118

scanner errors:
Error: USE_AFTER_FREE (CWE-416): [#def4]
nss-3.60.1/nss/cmd/crlutil/crlutil.c:389: freed_arg: "PORT_FreeArena_Util" frees "modArena".
nss-3.60.1/nss/cmd/crlutil/crlutil.c:455: double_free: Calling "PORT_FreeArena_Util" frees pointer "modArena" which has already been freed.
453| }
454| if (modArena && (!modCrl || modCrl->arena != modArena)) {
455|-> PORT_FreeArena(modArena, PR_FALSE);
456| }
457| if (modCrl)

Error: BAD_FREE (CWE-590): [#def6]
nss-3.60.1/nss/cmd/modutil/install-ds.c:1046: address_free: "PR_Free" frees address of "_this->forwardCompatible".
1044| Pk11Install_PlatformName_delete(&_this->forwardCompatible[i]);
1045| }
1046|-> PR_Free(&_this->forwardCompatible);
1047| _this->numForwardCompatible = 0;
1048| }

Error: USE_AFTER_FREE (CWE-416): [#def8]
nss-3.60.1/nss/cmd/signtool/javascript.c:1346: freed_arg: "PR_Free" frees "archiveDir".
nss-3.60.1/nss/cmd/signtool/javascript.c:1347: pass_freed_arg: Passing freed pointer "archiveDir" as an argument to "PR_smprintf".
1345| warningCount++;
1346| PR_Free(archiveDir);
1347|-> archiveDir = PR_smprintf("%s.arc", archiveDir);
1348| } else {
1349| PL_strcpy(archiveDir + strlen(archiveDir) - 4, ".arc");

Error: USE_AFTER_FREE (CWE-416): [#def9]
nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail".
nss-3.60.1/nss/cmd/signtool/javascript.c:1654: double_free: Calling "PR_Free" frees pointer "curitem" which has already been freed.
1652| }
1653| if (curitem) {
1654|-> PR_Free(curitem);
1655| }
1656| if (basedir) {

Error: USE_AFTER_FREE (CWE-416): [#def10]
nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage.
nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail".
nss-3.60.1/nss/cmd/signtool/javascript.c:1654: pass_freed_arg: Passing freed pointer "curitem" as an argument to "PR_Free".
1652| }
1653| if (curitem) {
1654|-> PR_Free(curitem);
1655| }
1656| if (basedir) {

Error: RESOURCE_LEAK (CWE-772): [#def12]
nss-3.60.1/nss/cmd/signtool/list.c:36: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util".
nss-3.60.1/nss/cmd/signtool/list.c:36: var_assign: Assigning: "ugly_list" = storage returned from "PORT_ZAlloc_Util(16UL)".
nss-3.60.1/nss/cmd/signtool/list.c:137: leaked_storage: Variable "ugly_list" going out of scope leaks the storage it points to.
135|
136| if (failed) {
137|-> return -1;
138| }
139| return 0;

Error: RESOURCE_LEAK (CWE-772): [#def13]
nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir".
nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)".
nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument.
nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir".
nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)".
nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "entry" going out of scope leaks the storage it points to.
nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
140| if (snprintf(filename, sizeof(filename), "%s/%s", path, entry->name) >= sizeof(filename)) {
141| errorCount++;
142|-> return -1;
143| }
144| if (rm_dash_r(filename))

Error: RESOURCE_LEAK (CWE-772): [#def14]
nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir".
nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)".
nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument.
nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir".
nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)".
nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "entry" going out of scope leaks the storage it points to.
nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
143| }
144| if (rm_dash_r(filename))
145|-> return -1;
146| }
147|

Error: RESOURCE_LEAK (CWE-772): [#def15]
nss-3.60.1/nss/cmd/signver/pk7print.c:325: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util".
nss-3.60.1/nss/cmd/signver/pk7print.c:325: var_assign: Assigning: "pk" = storage returned from "PORT_ZAlloc_Util(328UL)".
nss-3.60.1/nss/cmd/signver/pk7print.c:351: leaked_storage: Variable "pk" going out of scope leaks the storage it points to.
349| default:
350| fprintf(out, "%s=bad SPKI algorithm type\n", msg);
351|-> return 0;
352| }
353|

Error: RESOURCE_LEAK (CWE-772): [#def16]
nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: alloc_fn: Storage is returned from allocation function "PK11_GetSymKeyNickname".
nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: var_assign: Assigning: "name" = storage returned from "PK11_GetSymKeyNickname(symKey)".
nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:298: noescape: Resource "name ? name : " "" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:306: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
304| }
305| printf("\n");
306|-> }
307|
308| SECStatus

Error: USE_AFTER_FREE (CWE-416): [#def99]
nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:835: freed_arg: "PORT_FreeArena_Util" frees "arena".
nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:854: double_free: Calling "PORT_FreeArena_Util" frees pointer "arena" which has already been freed.
852| PKIX_CERTNAMECONSTRAINTS_DEBUG
853| ("\t\tCalling PORT_FreeArena).\n");
854|-> PORT_FreeArena(arena, PR_FALSE);
855| }
856| }

Error: VARARGS (CWE-237): [#def109]
nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:428: va_init: Initializing va_list "args".
nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:534: missing_va_end: "va_end" was not called for "args".
532| }
533|
534|-> PKIX_RETURN(STRING);
535| }
536|

Error: USE_AFTER_FREE (CWE-416): [#def112]
nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1099: alias: Assigning: "newSpecPtr" = "newSpec". Now both point to the same storage.
nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1156: freed_arg: "secmod_doDescCopy" frees "newSpecPtr".
nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1211: use_after_free: Using freed pointer "newSpec".
1209| /* no target found, return the newSpec */
1210| if (target == NULL) {
1211|-> return newSpec;
1212| }
1213|

Error: CPPCHECK_WARNING (CWE-562): [#def113]
nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'digestedData' that will be invalid when returning.
307| }
308| }
309|-> return cinfo;
310| }
311|

Error: CPPCHECK_WARNING (CWE-562): [#def114]
nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'encryptedData' that will be invalid when returning.
307| }
308| }
309|-> return cinfo;
310| }
311|

Error: CPPCHECK_WARNING (CWE-562): [#def115]
nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'envelopedData' that will be invalid when returning.
307| }
308| }
309|-> return cinfo;
310| }
311|

Error: CPPCHECK_WARNING (CWE-562): [#def116]
nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'genericData' that will be invalid when returning.
307| }
308| }
309|-> return cinfo;
310| }
311|

Error: CPPCHECK_WARNING (CWE-562): [#def117]
nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'signedData' that will be invalid when returning.
307| }
308| }
309|-> return cinfo;
310| }
311|

Error: USE_AFTER_FREE (CWE-416): [#def118]
nss-3.60.1/nss/lib/softoken/pkcs11.c:2671: freed_arg: "PORT_Realloc_Util" frees "oldNscSlotList".
nss-3.60.1/nss/lib/softoken/pkcs11.c:2674: use_after_free: Using freed pointer "oldNscSlotList".
2672| nscSlotListSize[index] * sizeof(CK_SLOT_ID));
2673| if (nscSlotList[index] == NULL) {
2674|-> nscSlotList[index] = oldNscSlotList;
2675| nscSlotListSize[index] = oldNscSlotListSize;
2676| return CKR_HOST_MEMORY;

Some of these are false positives, but they are reasonable issues for the scanners to flag, so scanner suppression comments, along with human reviewer comments will be added.

Differential Revision: https://phabricator.services.mozilla.com/D111339

13 files changed, 85 insertions, 36 deletions

diff --git a/cmd/crlutil/crlutil.c b/cmd/crlutil/crlutil.c
index be2e47a6c..2ce7b27d9 100644
--- a/cmd/crlutil/crlutil.c
+++ b/cmd/crlutil/crlutil.c
@@ -386,7 +386,6 @@ CreateModifiedCRLCopy(PLArenaPool *arena, CERTCertDBHandle *certHandle,
         rv = SECU_ReadDERFromFile(&crlDER, inFile, PR_FALSE, PR_FALSE);
         if (rv != SECSuccess) {
             SECU_PrintError(progName, "unable to read input file");
-            PORT_FreeArena(modArena, PR_FALSE);
             goto loser;
         }
 
diff --git a/cmd/lib/secutil.c b/cmd/lib/secutil.c
index b70a14172..7fb041ec7 100644
--- a/cmd/lib/secutil.c
+++ b/cmd/lib/secutil.c
@@ -1378,6 +1378,7 @@ secu_PrintECPublicKey(FILE *out, SECKEYPublicKey *pk, char *m, int level)
         (pk->u.ec.DEREncodedParams.data[0] == 0x06)) {
         curveOID.len = pk->u.ec.DEREncodedParams.data[1];
         curveOID.data = pk->u.ec.DEREncodedParams.data + 2;
+        curveOID.len = PR_MIN(curveOID.len, pk->u.ec.DEREncodedParams.len - 2);
         SECU_PrintObjectID(out, &curveOID, "Curve", level + 1);
     }
 }
diff --git a/cmd/modutil/install-ds.c b/cmd/modutil/install-ds.c
index a013d05a3..b14c28a0a 100644
--- a/cmd/modutil/install-ds.c
+++ b/cmd/modutil/install-ds.c
@@ -1034,7 +1034,7 @@ Pk11Install_Info_Cleanup(Pk11Install_Info* _this)
         for (i = 0; i < _this->numPlatforms; i++) {
             Pk11Install_Platform_delete(&_this->platforms[i]);
         }
-        PR_Free(&_this->platforms);
+        PR_Free(_this->platforms);
         _this->platforms = NULL;
         _this->numPlatforms = 0;
     }
@@ -1043,7 +1043,7 @@ Pk11Install_Info_Cleanup(Pk11Install_Info* _this)
         for (i = 0; i < _this->numForwardCompatible; i++) {
             Pk11Install_PlatformName_delete(&_this->forwardCompatible[i]);
         }
-        PR_Free(&_this->forwardCompatible);
+        PR_Free(_this->forwardCompatible);
         _this->numForwardCompatible = 0;
     }
 }
diff --git a/cmd/signtool/javascript.c b/cmd/signtool/javascript.c
index 58869aa61..87894b74a 100644
--- a/cmd/signtool/javascript.c
+++ b/cmd/signtool/javascript.c
@@ -1338,13 +1338,15 @@ extract_js(char *filename)
                 if ((PL_strlen(archiveDir) < 4) ||
                     PL_strcasecmp((archiveDir + strlen(archiveDir) - 4),
                                   ".jar")) {
+                    char *newArchiveDir = NULL;
                     PR_fprintf(errorFD,
                                "warning: ARCHIVE attribute should end in \".jar\" in tag"
                                " starting on %s:%d.\n",
                                filename, curitem->startLine);
                     warningCount++;
+                    newArchiveDir = PR_smprintf("%s.arc", archiveDir);
                     PR_Free(archiveDir);
-                    archiveDir = PR_smprintf("%s.arc", archiveDir);
+                    archiveDir = newArchiveDir;
                 } else {
                     PL_strcpy(archiveDir + strlen(archiveDir) - 4, ".arc");
                 }
@@ -1650,9 +1652,6 @@ loser:
     if (entityListTail) {
         PR_Free(entityListTail);
     }
-    if (curitem) {
-        PR_Free(curitem);
-    }
     if (basedir) {
         PR_Free(basedir);
     }
diff --git a/cmd/signtool/list.c b/cmd/signtool/list.c
index 70f62d2b1..dd42d8125 100644
--- a/cmd/signtool/list.c
+++ b/cmd/signtool/list.c
@@ -19,7 +19,6 @@ ListCerts(char *key, int list_certs)
 {
     int failed = 0;
     SECStatus rv;
-    char *ugly_list;
     CERTCertDBHandle *db;
 
     CERTCertificate *cert;
@@ -33,14 +32,6 @@ ListCerts(char *key, int list_certs)
     errlog.tail = NULL;
     errlog.count = 0;
 
-    ugly_list = PORT_ZAlloc(16);
-
-    if (ugly_list == NULL) {
-        out_of_memory();
-    }
-
-    *ugly_list = 0;
-
     db = CERT_GetDefaultCertDB();
 
     if (list_certs == 2) {
diff --git a/cmd/signtool/util.c b/cmd/signtool/util.c
index 49b7f3b05..ecd22e39c 100644
--- a/cmd/signtool/util.c
+++ b/cmd/signtool/util.c
@@ -138,8 +138,10 @@ rm_dash_r(char *path)
         /* Recursively delete all entries in the directory */
         while ((entry = PR_ReadDir(dir, PR_SKIP_BOTH)) != NULL) {
             sprintf(filename, "%s/%s", path, entry->name);
-            if (rm_dash_r(filename))
+            if (rm_dash_r(filename)) {
+                PR_CloseDir(dir);
                 return -1;
+            }
         }
 
         if (PR_CloseDir(dir) != PR_SUCCESS) {
diff --git a/cmd/signver/pk7print.c b/cmd/signver/pk7print.c
index deaaaf9e3..9ebf92088 100644
--- a/cmd/signver/pk7print.c
+++ b/cmd/signver/pk7print.c
@@ -311,40 +311,75 @@ sv_PrintDSAPublicKey(FILE *out, SECKEYPublicKey *pk, char *m)
     sv_PrintInteger(out, &pk->u.dsa.publicValue, "publicValue=");
 }
 
+void
+sv_PrintECDSAPublicKey(FILE *out, SECKEYPublicKey *pk, char *m)
+{
+    SECItem curve = { siBuffer, NULL, 0 };
+    if ((pk->u.ec.DEREncodedParams.len > 2) &&
+        (pk->u.ec.DEREncodedParams.data[0] == 0x06)) {
+        /* strip to just the oid for the curve */
+        curve.len = pk->u.ec.DEREncodedParams.data[1];
+        curve.data = pk->u.ec.DEREncodedParams.data + 2;
+        /* don't overflow the buffer */
+        curve.len = PR_MIN(curve.len, pk->u.ec.DEREncodedParams.len - 2);
+        fprintf(out, "%s", m);
+        sv_PrintObjectID(out, &curve, "curve=");
+    }
+    fprintf(out, "%s", m);
+    sv_PrintInteger(out, &pk->u.ec.publicValue, "publicValue=");
+}
+
 int
 sv_PrintSubjectPublicKeyInfo(FILE *out, PLArenaPool *arena,
                              CERTSubjectPublicKeyInfo *i, char *msg)
 {
-    SECKEYPublicKey *pk;
+    SECKEYPublicKey pk;
     int rv;
     char mm[200];
 
     sprintf(mm, "%s.publicKeyAlgorithm=", msg);
     sv_PrintAlgorithmID(out, &i->algorithm, mm);
 
-    pk = (SECKEYPublicKey *)PORT_ZAlloc(sizeof(SECKEYPublicKey));
-    if (!pk)
-        return PORT_GetError();
-
     DER_ConvertBitString(&i->subjectPublicKey);
     switch (SECOID_FindOIDTag(&i->algorithm.algorithm)) {
         case SEC_OID_PKCS1_RSA_ENCRYPTION:
-            rv = SEC_ASN1DecodeItem(arena, pk,
+        case SEC_OID_PKCS1_RSA_PSS_SIGNATURE:
+            rv = SEC_ASN1DecodeItem(arena, &pk,
                                     SEC_ASN1_GET(SECKEY_RSAPublicKeyTemplate),
                                     &i->subjectPublicKey);
             if (rv)
                 return rv;
             sprintf(mm, "%s.rsaPublicKey.", msg);
-            sv_PrintRSAPublicKey(out, pk, mm);
+            sv_PrintRSAPublicKey(out, &pk, mm);
             break;
         case SEC_OID_ANSIX9_DSA_SIGNATURE:
-            rv = SEC_ASN1DecodeItem(arena, pk,
+            rv = SEC_ASN1DecodeItem(arena, &pk,
                                     SEC_ASN1_GET(SECKEY_DSAPublicKeyTemplate),
                                     &i->subjectPublicKey);
             if (rv)
                 return rv;
+#ifdef notdef
+            /* SECKEY_PQGParamsTemplate is not yet exported form NSS */
+            rv = SEC_ASN1DecodeItem(arena, &pk.u.dsa.params,
+                                    SEC_ASN1_GET(SECKEY_PQGParamsTemplate),
+                                    &i->algorithm.parameters);
+            if (rv)
+                return rv;
+#endif
             sprintf(mm, "%s.dsaPublicKey.", msg);
-            sv_PrintDSAPublicKey(out, pk, mm);
+            sv_PrintDSAPublicKey(out, &pk, mm);
+            break;
+        case SEC_OID_ANSIX962_EC_PUBLIC_KEY:
+            rv = SECITEM_CopyItem(arena, &pk.u.ec.DEREncodedParams,
+                                  &i->algorithm.parameters);
+            if (rv)
+                return rv;
+            rv = SECITEM_CopyItem(arena, &pk.u.ec.publicValue,
+                                  &i->subjectPublicKey);
+            if (rv)
+                return rv;
+            sprintf(mm, "%s.ecdsaPublicKey.", msg);
+            sv_PrintECDSAPublicKey(out, &pk, mm);
             break;
         default:
             fprintf(out, "%s=bad SPKI algorithm type\n", msg);
diff --git a/cmd/symkeyutil/symkeyutil.c b/cmd/symkeyutil/symkeyutil.c
index 630433823..5f6355b8b 100644
--- a/cmd/symkeyutil/symkeyutil.c
+++ b/cmd/symkeyutil/symkeyutil.c
@@ -303,6 +303,7 @@ PrintKey(PK11SymKey *symKey)
         printf("<restricted>");
     }
     printf("\n");
+    PORT_Free(name);
 }
 
 SECStatus
diff --git a/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c b/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c
index affea8741..1d0b61bb4 100644
--- a/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c
+++ b/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c
@@ -829,10 +829,15 @@ pkix_pl_CertNameConstraints_Create(
 
         if (nssNameConstraints == NULL) {
                 *pNameConstraints = NULL;
+                 /* we free the arnea here because PKIX_ERROR_RECEIVED
+                  * may not be set. Setting arena to NULL makes sure
+                  * we don't try to free it again (and makes scanners
+                  * happy). */
                 if (arena){
                         PKIX_CERTNAMECONSTRAINTS_DEBUG
                                 ("\t\tCalling PORT_FreeArena).\n");
                         PORT_FreeArena(arena, PR_FALSE);
+                        arena = NULL;
                 }
                 goto cleanup;
         }
diff --git a/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c b/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c
index 8bfa2c996..167382466 100755
--- a/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c
+++ b/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c
@@ -439,7 +439,8 @@ PKIX_PL_Sprintf(
 
                                 tempString = va_arg(args, PKIX_PL_String *);
                                 if (tempString != NULL) {
-                                        PKIX_CHECK(PKIX_PL_String_GetEncoded
+                                        PKIX_CHECK_NO_GOTO(
+                                                PKIX_PL_String_GetEncoded
                                                 ((PKIX_PL_String*)
                                                 tempString,
                                                 PKIX_ESCASCII,
@@ -447,6 +448,12 @@ PKIX_PL_Sprintf(
                                                 &dummyLen,
                                                 plContext),
                                                 PKIX_STRINGGETENCODEDFAILED);
+                                        /* need to cleanup var args before
+                                         * we ditch out to cleanup. */
+                                        if (pkixErrorResult) {
+                                            va_end(args);
+                                            goto cleanup;
+                                        }
                                 } else {
                                         /* there may be a NULL in var_args */
                                         pArg = NULL;
diff --git a/lib/pk11wrap/pk11pars.c b/lib/pk11wrap/pk11pars.c
index edffa9e3a..23e5af32f 100644
--- a/lib/pk11wrap/pk11pars.c
+++ b/lib/pk11wrap/pk11pars.c
@@ -1038,8 +1038,8 @@ secmod_SetInternalKeySlotFlag(SECMODModule *mod, PRBool val)
  * try to expand the buffer with Realloc.
  */
 static char *
-secmod_doDescCopy(char *target, int *targetLen, const char *desc,
-                  int descLen, char *value)
+secmod_doDescCopy(char *target, char **base, int *baseLen,
+                  const char *desc, int descLen, char *value)
 {
     int diff, esc_len;
 
@@ -1048,12 +1048,14 @@ secmod_doDescCopy(char *target, int *targetLen, const char *desc,
     if (diff > 0) {
         /* we need to escape... expand newSpecPtr as well to make sure
          * we don't overflow it */
-        char *newPtr = PORT_Realloc(target, *targetLen * diff);
+        int offset = target - *base;
+        char *newPtr = PORT_Realloc(*base, *baseLen + diff);
         if (!newPtr) {
             return target; /* not enough space, just drop the whole copy */
         }
-        *targetLen += diff;
-        target = newPtr;
+        *baseLen += diff;
+        target = newPtr + offset;
+        *base = newPtr;
         value = NSSUTIL_Escape(value, '\"');
         if (value == NULL) {
             return target; /* couldn't escape value, just drop the copy */
@@ -1158,7 +1160,7 @@ secmod_ParseModuleSpecForTokens(PRBool convert, PRBool isFIPS,
                                       modulePrev = moduleSpec;
                                       if (!isFIPS) {
                                           newSpecPtr = secmod_doDescCopy(newSpecPtr,
-                                                                         &newSpecLen,
+                                                                         &newSpec, &newSpecLen,
                                                                          SECMOD_TOKEN_DESCRIPTION,
                                                                          sizeof(SECMOD_TOKEN_DESCRIPTION) - 1,
                                                                          tmp);
@@ -1169,7 +1171,7 @@ secmod_ParseModuleSpecForTokens(PRBool convert, PRBool isFIPS,
                                       modulePrev = moduleSpec; /* skip copying */
                                       if (!isFIPS) {
                                           newSpecPtr = secmod_doDescCopy(newSpecPtr,
-                                                                         &newSpecLen,
+                                                                         &newSpec, &newSpecLen,
                                                                          SECMOD_SLOT_DESCRIPTION,
                                                                          sizeof(SECMOD_SLOT_DESCRIPTION) - 1,
                                                                          tmp);
@@ -1180,7 +1182,7 @@ secmod_ParseModuleSpecForTokens(PRBool convert, PRBool isFIPS,
                                       modulePrev = moduleSpec; /* skip copying */
                                       if (isFIPS) {
                                           newSpecPtr = secmod_doDescCopy(newSpecPtr,
-                                                                         &newSpecLen,
+                                                                         &newSpec, &newSpecLen,
                                                                          SECMOD_TOKEN_DESCRIPTION,
                                                                          sizeof(SECMOD_TOKEN_DESCRIPTION) - 1,
                                                                          tmp);
@@ -1191,7 +1193,7 @@ secmod_ParseModuleSpecForTokens(PRBool convert, PRBool isFIPS,
                                       modulePrev = moduleSpec; /* skip copying */
                                       if (isFIPS) {
                                           newSpecPtr = secmod_doDescCopy(newSpecPtr,
-                                                                         &newSpecLen,
+                                                                         &newSpec, &newSpecLen,
                                                                          SECMOD_SLOT_DESCRIPTION,
                                                                          sizeof(SECMOD_SLOT_DESCRIPTION) - 1,
                                                                          tmp);
diff --git a/lib/smime/cmsutil.c b/lib/smime/cmsutil.c
index 713b94aac..844fd22a9 100644
--- a/lib/smime/cmsutil.c
+++ b/lib/smime/cmsutil.c
@@ -306,6 +306,10 @@ NSS_CMSContent_GetContentInfo(void *msg, SECOidTag type)
                 cinfo = &(c.genericData->contentInfo);
             }
     }
+    /* We are using a union as a form of 'safe casting'. This
+     * syntax confuses cppcheck, so tell it it's OK (and any human
+     * who happens along to verify any other scanner warnings) */
+    /* cppcheck-suppress returnDanglingLifetime */
     return cinfo;
 }
 
diff --git a/lib/softoken/pkcs11.c b/lib/softoken/pkcs11.c
index ba17298a9..eba27d208 100644
--- a/lib/softoken/pkcs11.c
+++ b/lib/softoken/pkcs11.c
@@ -2676,6 +2676,9 @@ sftk_RegisterSlot(SFTKSlot *slot, unsigned int moduleIndex)
         nscSlotList[index] = (CK_SLOT_ID *)PORT_Realloc(oldNscSlotList,
                                                         nscSlotListSize[index] * sizeof(CK_SLOT_ID));
         if (nscSlotList[index] == NULL) {
+            /* evidently coverity doesn't know realloc does not
+             * free var if it fails ! */
+            /* coverity [use_after_free : FALSE] */
             nscSlotList[index] = oldNscSlotList;
             nscSlotListSize[index] = oldNscSlotListSize;
             return CKR_HOST_MEMORY;