mirror of
https://git.libssh.org/projects/libssh.git
synced 2026-02-11 18:50:28 +09:00
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 <npocs@redhat.com> Reviewed-by: Jakub Jelen <jjelen@redhat.com> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This commit is contained in:
committed by
Andreas Schneider
parent
b733df6ddc
commit
05de7cb6ac
@@ -2291,8 +2291,12 @@ int pki_verify_data_signature(ssh_signature signature,
|
|||||||
unsigned char *raw_sig_data = NULL;
|
unsigned char *raw_sig_data = NULL;
|
||||||
unsigned int raw_sig_len;
|
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 rc = SSH_ERROR;
|
||||||
int evp_rc;
|
int ok;
|
||||||
|
|
||||||
if (pubkey == NULL || ssh_key_is_private(pubkey) || input == NULL ||
|
if (pubkey == NULL || ssh_key_is_private(pubkey) || input == NULL ||
|
||||||
signature == NULL || (signature->raw_sig == NULL
|
signature == NULL || (signature->raw_sig == NULL
|
||||||
@@ -2307,8 +2311,8 @@ int pki_verify_data_signature(ssh_signature signature,
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Check if public key and hash type are compatible */
|
/* Check if public key and hash type are compatible */
|
||||||
rc = pki_key_check_hash_compatible(pubkey, signature->hash_type);
|
ok = pki_key_check_hash_compatible(pubkey, signature->hash_type);
|
||||||
if (rc != SSH_OK) {
|
if (ok != SSH_OK) {
|
||||||
return SSH_ERROR;
|
return SSH_ERROR;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2351,8 +2355,8 @@ int pki_verify_data_signature(ssh_signature signature,
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Verify the signature */
|
/* Verify the signature */
|
||||||
evp_rc = EVP_DigestVerifyInit(ctx, NULL, md, NULL, pkey);
|
ok = EVP_DigestVerifyInit(ctx, NULL, md, NULL, pkey);
|
||||||
if (evp_rc != 1){
|
if (ok != 1){
|
||||||
SSH_LOG(SSH_LOG_TRACE,
|
SSH_LOG(SSH_LOG_TRACE,
|
||||||
"EVP_DigestVerifyInit() failed: %s",
|
"EVP_DigestVerifyInit() failed: %s",
|
||||||
ERR_error_string(ERR_get_error(), NULL));
|
ERR_error_string(ERR_get_error(), NULL));
|
||||||
@@ -2360,28 +2364,28 @@ int pki_verify_data_signature(ssh_signature signature,
|
|||||||
}
|
}
|
||||||
|
|
||||||
#ifdef HAVE_OPENSSL_EVP_DIGESTVERIFY
|
#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
|
#else
|
||||||
evp_rc = EVP_DigestVerifyUpdate(ctx, input, input_len);
|
ok = EVP_DigestVerifyUpdate(ctx, input, input_len);
|
||||||
if (evp_rc != 1) {
|
if (ok != 1) {
|
||||||
SSH_LOG(SSH_LOG_TRACE,
|
SSH_LOG(SSH_LOG_TRACE,
|
||||||
"EVP_DigestVerifyUpdate() failed: %s",
|
"EVP_DigestVerifyUpdate() failed: %s",
|
||||||
ERR_error_string(ERR_get_error(), NULL));
|
ERR_error_string(ERR_get_error(), NULL));
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
evp_rc = EVP_DigestVerifyFinal(ctx, raw_sig_data, raw_sig_len);
|
ok = EVP_DigestVerifyFinal(ctx, raw_sig_data, raw_sig_len);
|
||||||
#endif
|
#endif
|
||||||
if (evp_rc == 1) {
|
if (ok != 1) {
|
||||||
SSH_LOG(SSH_LOG_TRACE, "Signature valid");
|
|
||||||
rc = SSH_OK;
|
|
||||||
} else {
|
|
||||||
SSH_LOG(SSH_LOG_TRACE,
|
SSH_LOG(SSH_LOG_TRACE,
|
||||||
"Signature invalid: %s",
|
"Signature invalid: %s",
|
||||||
ERR_error_string(ERR_get_error(), NULL));
|
ERR_error_string(ERR_get_error(), NULL));
|
||||||
rc = SSH_ERROR;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
SSH_LOG(SSH_LOG_TRACE, "Signature valid");
|
||||||
|
rc = SSH_OK;
|
||||||
|
|
||||||
out:
|
out:
|
||||||
if (ctx != NULL) {
|
if (ctx != NULL) {
|
||||||
EVP_MD_CTX_free(ctx);
|
EVP_MD_CTX_free(ctx);
|
||||||
|
|||||||
Reference in New Issue
Block a user