err: clear flags better when clearing errors.

An attempt to clear an error with malloced data didn't clear the flags.
Now it clears all flags except the malloced flag.

Fixes #12530

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

diff --git a/crypto/err/err_local.h b/crypto/err/err_local.h
index 678f92dc0488d4a318846ce4bc8448ddedb206cd..d4e19dff241b0b034ddfedab2faf825d56795222 100644 (file)
--- a/crypto/err/err_local.h
+++ b/crypto/err/err_local.h
@@ -27,6 +27,7 @@ static ossl_inline void err_clear_data(ERR_STATE *es, size_t i, int deall)
             es->err_data_flags[i] = 0;
         } else if (es->err_data[i] != NULL) {
             es->err_data[i][0] = '\0';
+            es->err_data_flags[i] = ERR_TXT_MALLOCED;
         }
     } else {
         es->err_data[i] = NULL;
@@ -68,6 +69,8 @@ static ossl_inline void err_set_debug(ERR_STATE *es, size_t i,
 static ossl_inline void err_set_data(ERR_STATE *es, size_t i,
                                      void *data, size_t datasz, int flags)
 {
+    if ((es->err_data_flags[i] & ERR_TXT_MALLOCED) != 0)
+        OPENSSL_free(es->err_data[i]);
     es->err_data[i] = data;
     es->err_data_size[i] = datasz;
     es->err_data_flags[i] = flags;

diff --git a/test/errtest.c b/test/errtest.c
index e19501a03602870cbb329923283d652f9e2a0646..2d827ff89364cb3684c2b247857ba80ed987a2ed 100644 (file)
--- a/test/errtest.c
+++ b/test/errtest.c
@@ -287,6 +287,53 @@ static int test_marks(void)
     return 1;
 }
 
+static int test_clear_error(void)
+{
+    int flags = -1;
+    const char *data = NULL;
+    int res = 0;
+
+    /* Raise an error with data and clear it */
+    ERR_raise_data(0, 0, "hello %s", "world");
+    ERR_peek_error_data(&data, &flags);
+    if (!TEST_str_eq(data, "hello world")
+            || !TEST_int_eq(flags, ERR_TXT_STRING | ERR_TXT_MALLOCED))
+        goto err;
+    ERR_clear_error();
+
+    /* Raise a new error without data */
+    ERR_raise(0, 0);
+    ERR_peek_error_data(&data, &flags);
+    if (!TEST_str_eq(data, "")
+            || !TEST_int_eq(flags, ERR_TXT_MALLOCED))
+        goto err;
+    ERR_clear_error();
+
+    /* Raise a new error with data */
+    ERR_raise_data(0, 0, "goodbye %s world", "cruel");
+    ERR_peek_error_data(&data, &flags);
+    if (!TEST_str_eq(data, "goodbye cruel world")
+            || !TEST_int_eq(flags, ERR_TXT_STRING | ERR_TXT_MALLOCED))
+        goto err;
+    ERR_clear_error();
+
+    /*
+     * Raise a new error without data to check that the malloced storage
+     * is freed properly
+     */
+    ERR_raise(0, 0);
+    ERR_peek_error_data(&data, &flags);
+    if (!TEST_str_eq(data, "")
+            || !TEST_int_eq(flags, ERR_TXT_MALLOCED))
+        goto err;
+    ERR_clear_error();
+
+    res = 1;
+ err:
+     ERR_clear_error();
+    return res;
+}
+
 int setup_tests(void)
 {
     ADD_TEST(preserves_system_error);
@@ -296,5 +343,6 @@ int setup_tests(void)
     ADD_TEST(test_print_error_format);
 #endif
     ADD_TEST(test_marks);
+    ADD_TEST(test_clear_error);
     return 1;
 }