I had the OpenSSL SHA-3 functions free memory on error to match the API of the Gnulib crypto functions better, which do not need *_free_ctx. However, this is unintuitive since the caller will likely call *_free_ctx themselves on error like I did in lib/sha3-stream.c.
This patch removes the internal calls to free on errors to leave it up to the caller. It also protects against double frees by setting the pointer to NULL on sha3_free_ctx, and adds tests that would otherwise segfault. Hopefully I never have to deal with the EVP headache again. Collin
>From a82f1b76386f8b446ef8aebf4c5089d8db0aa30e Mon Sep 17 00:00:00 2001 Message-ID: <a82f1b76386f8b446ef8aebf4c5089d8db0aa30e.1757202255.git.collin.fu...@gmail.com> From: Collin Funk <[email protected]> Date: Sat, 6 Sep 2025 16:38:08 -0700 Subject: [PATCH] crypto/sha3-buffer, crypto/sha3: Protect against double frees (regr. today). * lib/sha3.c (sha3_free_ctx): Only free the EVP_MD_CTX if it is not NULL, and set it to NULL after freeing. (DEFINE_SHA3_INIT_CTX, sha3_finish_ctx, sha3_process_bytes): Leave freeing memory to the caller. * lib/sha3-stream.c (sha3_xxx_stream): Call sha3_free_ctx on success and if sha3_init_ctx fails. * tests/test-sha3-224-buffer.c (main): Add a test that would otherwise segfault without this patch. * tests/test-sha3-256-buffer.c (main): Likewise. * tests/test-sha3-384-buffer.c (main): Likewise. * tests/test-sha3-512-buffer.c (main): Likewise. --- ChangeLog | 13 +++++++++++++ lib/sha3-stream.c | 3 +++ lib/sha3.c | 9 +++++---- lib/sha3.h | 3 +-- tests/test-sha3-224-buffer.c | 11 +++++++++++ tests/test-sha3-256-buffer.c | 11 +++++++++++ tests/test-sha3-384-buffer.c | 11 +++++++++++ tests/test-sha3-512-buffer.c | 11 +++++++++++ 8 files changed, 66 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5133c402b3..49505aaaf9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2025-09-06 Collin Funk <[email protected]> + crypto/sha3-buffer, crypto/sha3: Protect against double frees (regr. today). + * lib/sha3.c (sha3_free_ctx): Only free the EVP_MD_CTX if it is not + NULL, and set it to NULL after freeing. + (DEFINE_SHA3_INIT_CTX, sha3_finish_ctx, sha3_process_bytes): Leave + freeing memory to the caller. + * lib/sha3-stream.c (sha3_xxx_stream): Call sha3_free_ctx on success and + if sha3_init_ctx fails. + * tests/test-sha3-224-buffer.c (main): Add a test that would otherwise + segfault without this patch. + * tests/test-sha3-256-buffer.c (main): Likewise. + * tests/test-sha3-384-buffer.c (main): Likewise. + * tests/test-sha3-512-buffer.c (main): Likewise. + crypto/sha3-buffer: Set errno when OpenSSL functions fail. * lib/sha3.c: Include <errno.h> (DEFINE_SHA3_INIT_CTX): Set errno to ENOMEM if function fails. diff --git a/lib/sha3-stream.c b/lib/sha3-stream.c index 43c056e22a..d071ea06f9 100644 --- a/lib/sha3-stream.c +++ b/lib/sha3-stream.c @@ -57,6 +57,7 @@ sha3_xxx_stream (FILE *stream, char const *alg, void *resblock, if (! init_ctx (&ctx)) { free (buffer); + sha3_free_ctx (ctx); return 1; } size_t sum; @@ -135,6 +136,8 @@ sha3_xxx_stream (FILE *stream, char const *alg, void *resblock, sha3_free_ctx (&ctx); return 1; } + + sha3_free_ctx (ctx); return 0; } diff --git a/lib/sha3.c b/lib/sha3.c index 93b53c8368..551b4822ff 100644 --- a/lib/sha3.c +++ b/lib/sha3.c @@ -347,7 +347,6 @@ sha3_process_block (const void *buffer, size_t len, struct sha3_ctx *ctx) if (result == 0) \ { \ errno = ENOMEM; \ - sha3_free_ctx (ctx); \ return false; \ } \ return true; \ @@ -361,7 +360,11 @@ DEFINE_SHA3_INIT_CTX (512) void sha3_free_ctx (struct sha3_ctx *ctx) { - EVP_MD_CTX_free (ctx->evp_ctx); + if (ctx->evp_ctx != NULL) + { + EVP_MD_CTX_free (ctx->evp_ctx); + ctx->evp_ctx = NULL; + } } void * @@ -375,7 +378,6 @@ void * sha3_finish_ctx (struct sha3_ctx *ctx, void *resbuf) { int result = EVP_DigestFinal_ex (ctx->evp_ctx, resbuf, NULL); - sha3_free_ctx (ctx); if (result == 0) { errno = EINVAL; @@ -408,7 +410,6 @@ sha3_process_bytes (const void *buffer, size_t len, struct sha3_ctx *ctx) if (result == 0) { errno = EINVAL; - sha3_free_ctx (ctx); return false; } return true; diff --git a/lib/sha3.h b/lib/sha3.h index 259f0d1f56..bd3a095482 100644 --- a/lib/sha3.h +++ b/lib/sha3.h @@ -68,8 +68,7 @@ extern bool sha3_256_init_ctx (struct sha3_ctx *ctx); extern bool sha3_384_init_ctx (struct sha3_ctx *ctx); extern bool sha3_512_init_ctx (struct sha3_ctx *ctx); -/* Free memory allocated by the init_structure. This is only required if - OpenSSL is used. */ +/* Free memory allocated by the init_structure. */ extern void sha3_free_ctx (struct sha3_ctx *ctx); /* Starting with the result of former calls of this function (or the diff --git a/tests/test-sha3-224-buffer.c b/tests/test-sha3-224-buffer.c index 0fe5f329d8..50512a7e52 100644 --- a/tests/test-sha3-224-buffer.c +++ b/tests/test-sha3-224-buffer.c @@ -89,5 +89,16 @@ main (void) free (large); } + /* Check that sha3_free_ctx can be called multiple times without + crashing. */ + { + struct sha3_ctx ctx; + if (sha3_224_init_ctx (&ctx)) + { + sha3_free_ctx (&ctx); + sha3_free_ctx (&ctx); + } + } + return 0; } diff --git a/tests/test-sha3-256-buffer.c b/tests/test-sha3-256-buffer.c index 8d47874adc..5877571813 100644 --- a/tests/test-sha3-256-buffer.c +++ b/tests/test-sha3-256-buffer.c @@ -90,5 +90,16 @@ main (void) free (large); } + /* Check that sha3_free_ctx can be called multiple times without + crashing. */ + { + struct sha3_ctx ctx; + if (sha3_256_init_ctx (&ctx)) + { + sha3_free_ctx (&ctx); + sha3_free_ctx (&ctx); + } + } + return 0; } diff --git a/tests/test-sha3-384-buffer.c b/tests/test-sha3-384-buffer.c index c85408ed4a..8f6ff7ef52 100644 --- a/tests/test-sha3-384-buffer.c +++ b/tests/test-sha3-384-buffer.c @@ -95,5 +95,16 @@ main (void) free (large); } + /* Check that sha3_free_ctx can be called multiple times without + crashing. */ + { + struct sha3_ctx ctx; + if (sha3_384_init_ctx (&ctx)) + { + sha3_free_ctx (&ctx); + sha3_free_ctx (&ctx); + } + } + return 0; } diff --git a/tests/test-sha3-512-buffer.c b/tests/test-sha3-512-buffer.c index 535dfde168..3be928507e 100644 --- a/tests/test-sha3-512-buffer.c +++ b/tests/test-sha3-512-buffer.c @@ -101,5 +101,16 @@ main (void) free (large); } + /* Check that sha3_free_ctx can be called multiple times without + crashing. */ + { + struct sha3_ctx ctx; + if (sha3_512_init_ctx (&ctx)) + { + sha3_free_ctx (&ctx); + sha3_free_ctx (&ctx); + } + } + return 0; } -- 2.51.0
