From b9a86d5dd8b5bd33be42390bcbb5121fe0ae71a1 Mon Sep 17 00:00:00 2001 From: Zhou Qingyang Date: Fri, 25 Mar 2022 20:28:32 +0800 Subject: [PATCH] Fix possible null pointer dereference of evp_pkey_get_legacy() evp_pkey_get_legacy() will return NULL on failure, however several uses of it or its wrappers does not check the return value of evp_pkey_get_legacy(), which could lead to NULL pointer dereference. Fix those possible bugs by adding NULL checking. Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/17967) --- crypto/dh/dh_ameth.c | 11 +++++++++-- crypto/ec/ecx_meth.c | 40 ++++++++++++++++++++++++++++++++++++++++ crypto/evp/p_dec.c | 8 ++++++-- crypto/evp/p_enc.c | 8 ++++++-- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/crypto/dh/dh_ameth.c b/crypto/dh/dh_ameth.c index b2ff8c3eb5e..47a6ab7d0c7 100644 --- a/crypto/dh/dh_ameth.c +++ b/crypto/dh/dh_ameth.c @@ -393,14 +393,21 @@ int DHparams_print(BIO *bp, const DH *x) static int dh_pkey_ctrl(EVP_PKEY *pkey, int op, long arg1, void *arg2) { + DH *dh; switch (op) { case ASN1_PKEY_CTRL_SET1_TLS_ENCPT: /* We should only be here if we have a legacy key */ if (!ossl_assert(evp_pkey_is_legacy(pkey))) return 0; - return ossl_dh_buf2key(evp_pkey_get0_DH_int(pkey), arg2, arg1); + dh = (DH *) evp_pkey_get0_DH_int(pkey); + if (dh == NULL) + return 0; + return ossl_dh_buf2key(dh, arg2, arg1); case ASN1_PKEY_CTRL_GET1_TLS_ENCPT: - return ossl_dh_key2buf(EVP_PKEY_get0_DH(pkey), arg2, 0, 1); + dh = (DH *) EVP_PKEY_get0_DH(pkey); + if (dh == NULL) + return 0; + return ossl_dh_key2buf(dh, arg2, 0, 1); default: return -2; } diff --git a/crypto/ec/ecx_meth.c b/crypto/ec/ecx_meth.c index f43c43c9c89..6e6e55e085d 100644 --- a/crypto/ec/ecx_meth.c +++ b/crypto/ec/ecx_meth.c @@ -809,6 +809,11 @@ static int pkey_ecd_digestsign25519(EVP_MD_CTX *ctx, unsigned char *sig, { const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (sig == NULL) { *siglen = ED25519_SIGSIZE; return 1; @@ -831,6 +836,11 @@ static int pkey_ecd_digestsign448(EVP_MD_CTX *ctx, unsigned char *sig, { const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (sig == NULL) { *siglen = ED448_SIGSIZE; return 1; @@ -853,6 +863,11 @@ static int pkey_ecd_digestverify25519(EVP_MD_CTX *ctx, const unsigned char *sig, { const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (siglen != ED25519_SIGSIZE) return 0; @@ -866,6 +881,11 @@ static int pkey_ecd_digestverify448(EVP_MD_CTX *ctx, const unsigned char *sig, { const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (siglen != ED448_SIGSIZE) return 0; @@ -1181,6 +1201,11 @@ static int s390x_pkey_ecd_digestsign25519(EVP_MD_CTX *ctx, const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); int rc; + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (sig == NULL) { *siglen = ED25519_SIGSIZE; return 1; @@ -1221,6 +1246,11 @@ static int s390x_pkey_ecd_digestsign448(EVP_MD_CTX *ctx, const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); int rc; + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (sig == NULL) { *siglen = ED448_SIGSIZE; return 1; @@ -1263,6 +1293,11 @@ static int s390x_pkey_ecd_digestverify25519(EVP_MD_CTX *ctx, } param; const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (siglen != ED25519_SIGSIZE) return 0; @@ -1290,6 +1325,11 @@ static int s390x_pkey_ecd_digestverify448(EVP_MD_CTX *ctx, } param; const ECX_KEY *edkey = evp_pkey_get_legacy(EVP_MD_CTX_get_pkey_ctx(ctx)->pkey); + if (edkey == NULL) { + ERR_raise(ERR_LIB_EC, EC_R_INVALID_KEY); + return 0; + } + if (siglen != ED448_SIGSIZE) return 0; diff --git a/crypto/evp/p_dec.c b/crypto/evp/p_dec.c index 7b33edecd55..29ea3f5fbcb 100644 --- a/crypto/evp/p_dec.c +++ b/crypto/evp/p_dec.c @@ -22,15 +22,19 @@ int EVP_PKEY_decrypt_old(unsigned char *key, const unsigned char *ek, int ekl, EVP_PKEY *priv) { int ret = -1; + RSA *rsa = NULL; if (EVP_PKEY_get_id(priv) != EVP_PKEY_RSA) { ERR_raise(ERR_LIB_EVP, EVP_R_PUBLIC_KEY_NOT_RSA); goto err; } + rsa = evp_pkey_get0_RSA_int(priv); + if (rsa == NULL) + goto err; + ret = - RSA_private_decrypt(ekl, ek, key, evp_pkey_get0_RSA_int(priv), - RSA_PKCS1_PADDING); + RSA_private_decrypt(ekl, ek, key, rsa, RSA_PKCS1_PADDING); err: return ret; } diff --git a/crypto/evp/p_enc.c b/crypto/evp/p_enc.c index d4db5951640..64e67514561 100644 --- a/crypto/evp/p_enc.c +++ b/crypto/evp/p_enc.c @@ -22,15 +22,19 @@ int EVP_PKEY_encrypt_old(unsigned char *ek, const unsigned char *key, int key_len, EVP_PKEY *pubk) { int ret = 0; + RSA *rsa = NULL; if (EVP_PKEY_get_id(pubk) != EVP_PKEY_RSA) { ERR_raise(ERR_LIB_EVP, EVP_R_PUBLIC_KEY_NOT_RSA); goto err; } + rsa = evp_pkey_get0_RSA_int(pubk); + if (rsa == NULL) + goto err; + ret = - RSA_public_encrypt(key_len, key, ek, evp_pkey_get0_RSA_int(pubk), - RSA_PKCS1_PADDING); + RSA_public_encrypt(key_len, key, ek, rsa, RSA_PKCS1_PADDING); err: return ret; } -- 2.47.2