From d5f854291336c96a3d2379ecc8c29f00ef516ad9 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 27 Aug 2019 10:12:34 +0200 Subject: [PATCH] Coverty fixes for MACs Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/9700) --- apps/list.c | 12 ++++++------ crypto/cmac/cmac.c | 22 ++++++++++++++++------ providers/common/macs/cmac_prov.c | 3 ++- test/evp_test.c | 8 +++++--- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/apps/list.c b/apps/list.c index 0d93f5498f..46a3c29051 100644 --- a/apps/list.c +++ b/apps/list.c @@ -133,8 +133,8 @@ static void collect_ciphers(EVP_CIPHER *cipher, void *stack) { STACK_OF(EVP_CIPHER) *cipher_stack = stack; - sk_EVP_CIPHER_push(cipher_stack, cipher); - EVP_CIPHER_up_ref(cipher); + if (sk_EVP_CIPHER_push(cipher_stack, cipher) > 0) + EVP_CIPHER_up_ref(cipher); } static void list_ciphers(void) @@ -196,8 +196,8 @@ static void collect_digests(EVP_MD *md, void *stack) { STACK_OF(EVP_MD) *digest_stack = stack; - sk_EVP_MD_push(digest_stack, md); - EVP_MD_up_ref(md); + if (sk_EVP_MD_push(digest_stack, md) > 0) + EVP_MD_up_ref(md); } static void list_digests(void) @@ -245,8 +245,8 @@ static void collect_macs(EVP_MAC *mac, void *stack) { STACK_OF(EVP_MAC) *mac_stack = stack; - sk_EVP_MAC_push(mac_stack, mac); - EVP_MAC_up_ref(mac); + if (sk_EVP_MAC_push(mac_stack, mac) > 0) + EVP_MAC_up_ref(mac); } static void list_macs(void) diff --git a/crypto/cmac/cmac.c b/crypto/cmac/cmac.c index 79936a59a2..b1be991f87 100644 --- a/crypto/cmac/cmac.c +++ b/crypto/cmac/cmac.c @@ -87,11 +87,13 @@ void CMAC_CTX_free(CMAC_CTX *ctx) int CMAC_CTX_copy(CMAC_CTX *out, const CMAC_CTX *in) { int bl; + if (in->nlast_block == -1) return 0; + if ((bl = EVP_CIPHER_CTX_block_size(in->cctx)) < 0) + return 0; if (!EVP_CIPHER_CTX_copy(out->cctx, in->cctx)) return 0; - bl = EVP_CIPHER_CTX_block_size(in->cctx); memcpy(out->k1, in->k1, bl); memcpy(out->k2, in->k2, bl); memcpy(out->tbl, in->tbl, bl); @@ -104,6 +106,7 @@ int CMAC_Init(CMAC_CTX *ctx, const void *key, size_t keylen, const EVP_CIPHER *cipher, ENGINE *impl) { static const unsigned char zero_iv[EVP_MAX_BLOCK_LENGTH] = { 0 }; + /* All zeros means restart */ if (!key && !cipher && !impl && keylen == 0) { /* Not initialised */ @@ -121,13 +124,15 @@ int CMAC_Init(CMAC_CTX *ctx, const void *key, size_t keylen, /* Non-NULL key means initialisation complete */ if (key) { int bl; + if (!EVP_CIPHER_CTX_cipher(ctx->cctx)) return 0; if (!EVP_CIPHER_CTX_set_key_length(ctx->cctx, keylen)) return 0; if (!EVP_EncryptInit_ex(ctx->cctx, NULL, NULL, key, zero_iv)) return 0; - bl = EVP_CIPHER_CTX_block_size(ctx->cctx); + if ((bl = EVP_CIPHER_CTX_block_size(ctx->cctx)) < 0) + return 0; if (!EVP_Cipher(ctx->cctx, ctx->tbl, zero_iv, bl)) return 0; make_kn(ctx->k1, ctx->tbl, bl); @@ -146,15 +151,18 @@ int CMAC_Init(CMAC_CTX *ctx, const void *key, size_t keylen, int CMAC_Update(CMAC_CTX *ctx, const void *in, size_t dlen) { const unsigned char *data = in; - size_t bl; + int bl; + if (ctx->nlast_block == -1) return 0; if (dlen == 0) return 1; - bl = EVP_CIPHER_CTX_block_size(ctx->cctx); + if ((bl = EVP_CIPHER_CTX_block_size(ctx->cctx)) < 0) + return 0; /* Copy into partial block if we need to */ if (ctx->nlast_block > 0) { size_t nleft; + nleft = bl - ctx->nlast_block; if (dlen < nleft) nleft = dlen; @@ -170,7 +178,7 @@ int CMAC_Update(CMAC_CTX *ctx, const void *in, size_t dlen) return 0; } /* Encrypt all but one of the complete blocks left */ - while (dlen > bl) { + while (dlen > (size_t)bl) { if (!EVP_Cipher(ctx->cctx, ctx->tbl, data, bl)) return 0; dlen -= bl; @@ -186,9 +194,11 @@ int CMAC_Update(CMAC_CTX *ctx, const void *in, size_t dlen) int CMAC_Final(CMAC_CTX *ctx, unsigned char *out, size_t *poutlen) { int i, bl, lb; + if (ctx->nlast_block == -1) return 0; - bl = EVP_CIPHER_CTX_block_size(ctx->cctx); + if ((bl = EVP_CIPHER_CTX_block_size(ctx->cctx)) < 0) + return 0; *poutlen = (size_t)bl; if (!out) return 1; diff --git a/providers/common/macs/cmac_prov.c b/providers/common/macs/cmac_prov.c index 64ecba2b37..693423130d 100644 --- a/providers/common/macs/cmac_prov.c +++ b/providers/common/macs/cmac_prov.c @@ -66,8 +66,9 @@ static void *cmac_new(void *provctx) || (macctx->ctx = CMAC_CTX_new()) == NULL) { OPENSSL_free(macctx); macctx = NULL; + } else { + macctx->provctx = provctx; } - macctx->provctx = provctx; return macctx; } diff --git a/test/evp_test.c b/test/evp_test.c index 17b9fc0dfb..fd50c71354 100644 --- a/test/evp_test.c +++ b/test/evp_test.c @@ -1228,9 +1228,11 @@ static int mac_test_run_mac(EVP_TEST *t) if (tmpval != NULL) *tmpval++ = '\0'; - if (!OSSL_PARAM_allocate_from_text(¶ms[params_n], defined_params, - tmpkey, tmpval, - strlen(tmpval))) { + if (tmpval == NULL + || !OSSL_PARAM_allocate_from_text(¶ms[params_n], + defined_params, + tmpkey, tmpval, + strlen(tmpval))) { OPENSSL_free(tmpkey); t->err = "MAC_PARAM_ERROR"; goto err; -- 2.25.1