From e8dfbb85a28514e1f869dac3000c6cec6cb8d08d Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Mon, 24 Apr 2023 11:51:36 +0200 Subject: [PATCH] CVE-2023-2283:pki_crypto: Fix possible authentication bypass The return value is changed by the call to pki_key_check_hash_compatible causing the possibility of returning SSH_OK if memory allocation error happens later in the function. The assignment of SSH_ERROR if the verification fails is no longer needed, because the value of the variable is already SSH_ERROR. Signed-off-by: Norbert Pocs Reviewed-by: Jakub Jelen Reviewed-by: Andreas Schneider Conflict:NA Reference:https://gitlab.com/libssh/libssh-mirror/commit/e8dfbb85a28514e1f869dac3000c6cec6cb8d08d --- src/pki_crypto.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/pki_crypto.c b/src/pki_crypto.c index 013f569e..635b82cb 100644 --- a/src/pki_crypto.c +++ b/src/pki_crypto.c @@ -3175,8 +3175,12 @@ int pki_verify_data_signature(ssh_signature signature, unsigned char *raw_sig_data = NULL; unsigned int raw_sig_len; + /* Function return code + * Do not change this variable throughout the function until the signature + * is successfully verified! + */ int rc = SSH_ERROR; - int evp_rc; + int ok; if (pubkey == NULL || ssh_key_is_private(pubkey) || input == NULL || signature == NULL || (signature->raw_sig == NULL @@ -3191,8 +3195,8 @@ int pki_verify_data_signature(ssh_signature signature, } /* Check if public key and hash type are compatible */ - rc = pki_key_check_hash_compatible(pubkey, signature->hash_type); - if (rc != SSH_OK) { + ok = pki_key_check_hash_compatible(pubkey, signature->hash_type); + if (ok != SSH_OK) { return SSH_ERROR; } @@ -3237,8 +3241,8 @@ int pki_verify_data_signature(ssh_signature signature, } /* Verify the signature */ - evp_rc = EVP_DigestVerifyInit(ctx, NULL, md, NULL, pkey); - if (evp_rc != 1){ + ok = EVP_DigestVerifyInit(ctx, NULL, md, NULL, pkey); + if (ok != 1){ SSH_LOG(SSH_LOG_TRACE, "EVP_DigestVerifyInit() failed: %s", ERR_error_string(ERR_get_error(), NULL)); @@ -3246,28 +3250,28 @@ int pki_verify_data_signature(ssh_signature signature, } #ifdef HAVE_OPENSSL_EVP_DIGESTVERIFY - evp_rc = EVP_DigestVerify(ctx, raw_sig_data, raw_sig_len, input, input_len); + ok = EVP_DigestVerify(ctx, raw_sig_data, raw_sig_len, input, input_len); #else - evp_rc = EVP_DigestVerifyUpdate(ctx, input, input_len); - if (evp_rc != 1) { + ok = EVP_DigestVerifyUpdate(ctx, input, input_len); + if (ok != 1) { SSH_LOG(SSH_LOG_TRACE, "EVP_DigestVerifyUpdate() failed: %s", ERR_error_string(ERR_get_error(), NULL)); goto out; } - evp_rc = EVP_DigestVerifyFinal(ctx, raw_sig_data, raw_sig_len); + ok = EVP_DigestVerifyFinal(ctx, raw_sig_data, raw_sig_len); #endif - if (evp_rc == 1) { - SSH_LOG(SSH_LOG_TRACE, "Signature valid"); - rc = SSH_OK; - } else { + if (ok != 1) { SSH_LOG(SSH_LOG_TRACE, "Signature invalid: %s", ERR_error_string(ERR_get_error(), NULL)); - rc = SSH_ERROR; + goto out; } + SSH_LOG(SSH_LOG_TRACE, "Signature valid"); + rc = SSH_OK; + out: if (ctx != NULL) { EVP_MD_CTX_free(ctx); -- 2.33.0