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: Andreas Schneider <asn@cryptomilk.org>

Author: Norbert Pocs

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,37 +3241,37 @@ 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));
         goto out;
     }
 
 #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);