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 #15667)
openssl · Jun 10, 2021 · 5a9dbfc · 5a9dbfc
1 parent 5ac6d7d
commit 5a9dbfc
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git 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
@@ -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;
 }