If the init_ctx functions fail, it’s natural for callers to immediately fail too. Change the init_ctx functions to not leak when failing. This doesn’t invalidate any callers that free if the init functions fail, as that free now becomes a no-op. * lib/sha3.c (sha3_##SIZE##_init_ctx): Before failing, free any storage that was allocated before failure was discovered. * tests/test-sha3-224-buffer.c: * tests/test-sha3-256-buffer.c: * tests/test-sha3-384-buffer.c: * tests/test-sha3-512-buffer.c: (check, main): Test more cases of multiple frees. Also, fix a memory leak. --- ChangeLog | 14 ++++++++++++++ lib/sha3.c | 20 +++++++------------- tests/test-sha3-224-buffer.c | 9 ++++----- tests/test-sha3-256-buffer.c | 9 ++++----- tests/test-sha3-384-buffer.c | 9 ++++----- tests/test-sha3-512-buffer.c | 9 ++++----- 6 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/ChangeLog b/ChangeLog index 18c2dd56d7..9144f8a0c2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,19 @@ 2026-02-22 Paul Eggert <[email protected]> + crypto/sha3: Don’t leak if init fails and no free + If the init_ctx functions fail, it’s natural for callers to + immediately fail too. Change the init_ctx functions to not leak + when failing. This doesn’t invalidate any callers that free + if the init functions fail, as that free now becomes a no-op. + * lib/sha3.c (sha3_##SIZE##_init_ctx): Before failing, + free any storage that was allocated before failure was discovered. + * tests/test-sha3-224-buffer.c: + * tests/test-sha3-256-buffer.c: + * tests/test-sha3-384-buffer.c: + * tests/test-sha3-512-buffer.c: + (check, main): Test more cases of multiple frees. + Also, fix a memory leak. + crypto/sha3: fix sha3_read_ctx reset bug * lib/sha3.c (sha3_read_ctx): When using OpenSSL, don’t update the internal context; we’re supposed to read it, not write it. diff --git a/lib/sha3.c b/lib/sha3.c index ec893fc003..dec3b94b9a 100644 --- a/lib/sha3.c +++ b/lib/sha3.c @@ -337,21 +337,15 @@ sha3_process_block (const void *buffer, size_t len, struct sha3_ctx *ctx) bool \ sha3_##SIZE##_init_ctx (struct sha3_ctx *ctx) \ { \ - int result; \ - ctx->evp_ctx = EVP_MD_CTX_new (); \ - if (ctx->evp_ctx == NULL) \ + EVP_MD_CTX *evp_ctx = EVP_MD_CTX_new (); \ + if (evp_ctx && ! EVP_DigestInit_ex (evp_ctx, EVP_sha3_##SIZE (), NULL)) \ { \ - errno = ENOMEM; \ - return false; \ + EVP_MD_CTX_free (evp_ctx); \ + evp_ctx = NULL; \ } \ - result = EVP_DigestInit_ex (ctx->evp_ctx, EVP_sha3_##SIZE (), \ - NULL); \ - if (result == 0) \ - { \ - errno = ENOMEM; \ - return false; \ - } \ - return true; \ + ctx->evp_ctx = evp_ctx; \ + errno = ENOMEM; /* OK to set errno even if successful. */ \ + return !!evp_ctx; \ } DEFINE_SHA3_INIT_CTX (224) diff --git a/tests/test-sha3-224-buffer.c b/tests/test-sha3-224-buffer.c index 33190be91b..1a96e609a9 100644 --- a/tests/test-sha3-224-buffer.c +++ b/tests/test-sha3-224-buffer.c @@ -78,6 +78,7 @@ check (char const *message, size_t len, char const *expect) sha3_process_bytes (message + part, SHA3_224_BLOCK_SIZE - part, &ctx); char buf2[SHA3_224_DIGEST_SIZE]; sha3_finish_ctx (&ctx, buf2); + sha3_free_ctx (&ctx); if (mismatch (sha3_224_buffer (message, SHA3_224_BLOCK_SIZE, buf), buf2)) { failed = 1; @@ -119,11 +120,9 @@ main (void) crashing. */ { struct sha3_ctx ctx; - if (sha3_224_init_ctx (&ctx)) - { - sha3_free_ctx (&ctx); - sha3_free_ctx (&ctx); - } + 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 4f03654c2b..27aa8d7573 100644 --- a/tests/test-sha3-256-buffer.c +++ b/tests/test-sha3-256-buffer.c @@ -79,6 +79,7 @@ check (char const *message, size_t len, char const *expect) sha3_process_bytes (message + part, SHA3_256_BLOCK_SIZE - part, &ctx); char buf2[SHA3_256_DIGEST_SIZE]; sha3_finish_ctx (&ctx, buf2); + sha3_free_ctx (&ctx); if (mismatch (sha3_256_buffer (message, SHA3_256_BLOCK_SIZE, buf), buf2)) { failed = 1; @@ -120,11 +121,9 @@ main (void) crashing. */ { struct sha3_ctx ctx; - if (sha3_256_init_ctx (&ctx)) - { - sha3_free_ctx (&ctx); - sha3_free_ctx (&ctx); - } + sha3_224_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 c2cbd4f8d9..9a9fdef76a 100644 --- a/tests/test-sha3-384-buffer.c +++ b/tests/test-sha3-384-buffer.c @@ -83,6 +83,7 @@ check (char const *message, size_t len, char const *expect) sha3_process_bytes (message + part, SHA3_384_BLOCK_SIZE - part, &ctx); char buf2[SHA3_384_DIGEST_SIZE]; sha3_finish_ctx (&ctx, buf2); + sha3_free_ctx (&ctx); if (mismatch (sha3_384_buffer (message, SHA3_384_BLOCK_SIZE, buf), buf2)) { failed = 1; @@ -125,11 +126,9 @@ main (void) crashing. */ { struct sha3_ctx ctx; - if (sha3_384_init_ctx (&ctx)) - { - sha3_free_ctx (&ctx); - sha3_free_ctx (&ctx); - } + sha3_224_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 186cd2da00..55af231c54 100644 --- a/tests/test-sha3-512-buffer.c +++ b/tests/test-sha3-512-buffer.c @@ -87,6 +87,7 @@ check (char const *message, size_t len, char const *expect) sha3_process_bytes (message + part, SHA3_512_BLOCK_SIZE - part, &ctx); char buf2[SHA3_512_DIGEST_SIZE]; sha3_finish_ctx (&ctx, buf2); + sha3_free_ctx (&ctx); if (mismatch (sha3_512_buffer (message, SHA3_512_BLOCK_SIZE, buf), buf2)) { failed = 1; @@ -131,11 +132,9 @@ main (void) crashing. */ { struct sha3_ctx ctx; - if (sha3_512_init_ctx (&ctx)) - { - sha3_free_ctx (&ctx); - sha3_free_ctx (&ctx); - } + sha3_224_init_ctx (&ctx); + sha3_free_ctx (&ctx); + sha3_free_ctx (&ctx); } return 0; -- 2.51.0
