Fix stack corruption in ui_read

This is an alternative to #20893

Additionally this fixes also a possible issue in UI_UTIL_read_pw:

When UI_new returns NULL, the result code would still be zero
as if UI_UTIL_read_pw succeeded, but the password buffer is left
uninitialized, with subsequent possible stack corruption or worse.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20957)

(cherry picked from commit a64c48cff88e032cf9513578493c4536df725a22)

diff --git a/crypto/ui/ui_lib.c b/crypto/ui/ui_lib.c
index 1ff8c6fa35f342482e65fbbe160ca066f545d365..dbd2722bdabfed4b7cb2ae8377c626e67c2f857c 100644 (file)
--- a/crypto/ui/ui_lib.c
+++ b/crypto/ui/ui_lib.c
@@ -528,6 +528,10 @@ int UI_process(UI *ui)
                 ok = 0;
                 break;
             }
+        } else {
+            ui->flags &= ~UI_FLAG_REDOABLE;
+            ok = -2;
+            goto err;
         }
     }
 

diff --git a/crypto/ui/ui_util.c b/crypto/ui/ui_util.c
index 80297969ab1d660502975344f7995517ad0f311f..e26c1b5d25d5016463a522aa7220bab4cb82308e 100644 (file)
--- a/crypto/ui/ui_util.c
+++ b/crypto/ui/ui_util.c
@@ -32,7 +32,7 @@ int UI_UTIL_read_pw_string(char *buf, int length, const char *prompt,
 int UI_UTIL_read_pw(char *buf, char *buff, int size, const char *prompt,
                     int verify)
 {
-    int ok = 0;
+    int ok = -2;
     UI *ui;
 
     if (size < 1)
@@ -47,8 +47,6 @@ int UI_UTIL_read_pw(char *buf, char *buff, int size, const char *prompt,
             ok = UI_process(ui);
         UI_free(ui);
     }
-    if (ok > 0)
-        ok = 0;
     return ok;
 }
 

diff --git a/test/evp_extra_test2.c b/test/evp_extra_test2.c
index 88f1fc5b3aab208bdf13d2c43d1f3d54d33d904e..4d456053abb3a63bacd9407211ade4c0d37e69fb 100644 (file)
--- a/test/evp_extra_test2.c
+++ b/test/evp_extra_test2.c
@@ -23,6 +23,7 @@
 #include <openssl/rsa.h>
 #include <openssl/dh.h>
 #include <openssl/core_names.h>
+#include <openssl/ui.h>
 
 #include "testutil.h"
 #include "internal/nelem.h"
@@ -669,6 +670,52 @@ static int test_PEM_read_bio_negative(int testid)
     return ok;
 }
 
+static int test_PEM_read_bio_negative_wrong_password(int testid)
+{
+    int ok = 0;
+    OSSL_PROVIDER *provider = OSSL_PROVIDER_load(NULL, "default");
+    EVP_PKEY *read_pkey = NULL;
+    EVP_PKEY *write_pkey = EVP_RSA_gen(1024);
+    BIO *key_bio = BIO_new(BIO_s_mem());
+    const UI_METHOD *undo_ui_method = NULL;
+    const UI_METHOD *ui_method = NULL;
+    if (testid > 0)
+        ui_method = UI_null();
+
+    if (!TEST_ptr(provider))
+        goto err;
+    if (!TEST_ptr(key_bio))
+        goto err;
+    if (!TEST_ptr(write_pkey))
+        goto err;
+    undo_ui_method = UI_get_default_method();
+    UI_set_default_method(ui_method);
+
+    if (/* Output Encrypted private key in PEM form */
+        !TEST_true(PEM_write_bio_PrivateKey(key_bio, write_pkey, EVP_aes_256_cbc(),
+                                           NULL, 0, NULL, "pass")))
+        goto err;
+
+    ERR_clear_error();
+    read_pkey = PEM_read_bio_PrivateKey(key_bio, NULL, NULL, NULL);
+    if (!TEST_ptr_null(read_pkey))
+        goto err;
+
+    if (!TEST_int_eq(ERR_GET_REASON(ERR_get_error()), PEM_R_PROBLEMS_GETTING_PASSWORD))
+        goto err;
+    ok = 1;
+
+ err:
+    test_openssl_errors();
+    EVP_PKEY_free(read_pkey);
+    EVP_PKEY_free(write_pkey);
+    BIO_free(key_bio);
+    OSSL_PROVIDER_unload(provider);
+    UI_set_default_method(undo_ui_method);
+
+    return ok;
+}
+
 static int do_fromdata_key_is_equal(const OSSL_PARAM params[],
                                     const EVP_PKEY *expected, const char *type)
 {
@@ -1227,6 +1274,7 @@ int setup_tests(void)
     ADD_TEST(test_pkcs8key_nid_bio);
 #endif
     ADD_ALL_TESTS(test_PEM_read_bio_negative, OSSL_NELEM(keydata));
+    ADD_ALL_TESTS(test_PEM_read_bio_negative_wrong_password, 2);
     ADD_TEST(test_rsa_pss_sign);
     ADD_TEST(test_evp_md_ctx_dup);
     ADD_TEST(test_evp_md_ctx_copy);