diff --git a/cmd/crlutil/crlutil.c b/cmd/crlutil/crlutil.c index be2e47a6c4..2ce7b27d95 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 b70a14172c..7fb041ec7d 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 a013d05a33..b14c28a0ac 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 58869aa619..87894b74af 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 @@ extract_js(char *filename) 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 70f62d2b1a..dd42d81258 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 49b7f3b057..ecd22e39c6 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 deaaaf9e32..9ebf92088c 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 630433823c..5f6355b8b6 100644 --- a/cmd/symkeyutil/symkeyutil.c +++ b/cmd/symkeyutil/symkeyutil.c @@ -303,6 +303,7 @@ PrintKey(PK11SymKey *symKey) printf(""); } 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 affea8741d..1d0b61bb44 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 8bfa2c9966..167382466e 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 edffa9e3ac..23e5af32f0 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 713b94aacb..844fd22a93 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 ba17298a9b..eba27d2080 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;