commit fdaf9ca025e08564e2ff5b49ce1de39086943c5f
parent 7d8c068e5ea3a574155fa6950f7d19fa67c49e62
Author: Christian Grothoff <christian@grothoff.org>
Date: Tue, 28 Jul 2026 19:53:28 +0200
fix memory leaks
Diffstat:
3 files changed, 97 insertions(+), 20 deletions(-)
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
@@ -648,6 +648,45 @@ MHD_TLS_init (struct MHD_Daemon *daemon)
}
+/**
+ * Release the TLS resources that #MHD_start_daemon_va() may already have
+ * allocated when it decides to fail.
+ *
+ * The daemon object is not usable at that point, so #MHD_stop_daemon()
+ * (which is what frees these on the success path) cannot be called; each
+ * failure exit therefore has to do it, and every one of them used to
+ * forget the Diffie-Hellman parameters. Those are created by MHD itself
+ * from the PEM blob of #MHD_OPTION_HTTPS_MEM_DHPARAMS, so the
+ * application has no handle to free either.
+ *
+ * @param[in,out] daemon the half-initialised daemon
+ */
+static void
+tls_cleanup_failed_start (struct MHD_Daemon *daemon)
+{
+ if (daemon->have_dhparams)
+ {
+ gnutls_dh_params_deinit (daemon->https_mem_dhparams);
+ daemon->have_dhparams = false;
+ }
+ if (NULL != daemon->priority_cache)
+ {
+ gnutls_priority_deinit (daemon->priority_cache);
+ daemon->priority_cache = NULL;
+ }
+ if (NULL != daemon->x509_cred)
+ {
+ gnutls_certificate_free_credentials (daemon->x509_cred);
+ daemon->x509_cred = NULL;
+ }
+ if (NULL != daemon->psk_cred)
+ {
+ gnutls_psk_free_server_credentials (daemon->psk_cred);
+ daemon->psk_cred = NULL;
+ }
+}
+
+
#endif /* HTTPS_SUPPORT */
@@ -7878,9 +7917,7 @@ MHD_start_daemon_va (unsigned int flags,
ap))
{
#ifdef HTTPS_SUPPORT
- if ( (0 != (*pflags & MHD_USE_TLS)) &&
- (NULL != daemon->priority_cache) )
- gnutls_priority_deinit (daemon->priority_cache);
+ tls_cleanup_failed_start (daemon);
#endif /* HTTPS_SUPPORT */
free (interim_params);
free (daemon);
@@ -7891,6 +7928,9 @@ MHD_start_daemon_va (unsigned int flags,
&addrlen,
interim_params))
{
+#ifdef HTTPS_SUPPORT
+ tls_cleanup_failed_start (daemon);
+#endif /* HTTPS_SUPPORT */
free (interim_params);
free (daemon);
return NULL;
@@ -7906,6 +7946,7 @@ MHD_start_daemon_va (unsigned int flags,
MHD_DLOG (daemon,
_ ("Failed to initialise GnuTLS priorities.\n"));
#endif /* HAVE_MESSAGES */
+ tls_cleanup_failed_start (daemon);
free (daemon);
return NULL;
}
@@ -9009,14 +9050,7 @@ free_and_fail:
#endif
#endif
#ifdef HTTPS_SUPPORT
- if (0 != (*pflags & MHD_USE_TLS))
- {
- gnutls_priority_deinit (daemon->priority_cache);
- if (daemon->x509_cred)
- gnutls_certificate_free_credentials (daemon->x509_cred);
- if (daemon->psk_cred)
- gnutls_psk_free_server_credentials (daemon->psk_cred);
- }
+ tls_cleanup_failed_start (daemon);
#endif /* HTTPS_SUPPORT */
if (MHD_ITC_IS_VALID_ (daemon->itc))
MHD_itc_destroy_chk_ (daemon->itc);
diff --git a/src/microhttpd/memorypool.c b/src/microhttpd/memorypool.c
@@ -548,6 +548,17 @@ MHD_pool_reallocate (struct MemoryPool *pool,
mhd_assert (NULL == __asan_region_is_poisoned (old, old_size));
#endif /* MHD_ASAN_POISON_ACTIVE && HAVE___ASAN_REGION_IS_POISONED */
+ /* No block can ever be larger than the pool, so reject that here once
+ rather than in each of the paths below. This is what stops a
+ wrapping @a new_size: both remaining size tests are made on values
+ that ROUND_TO_ALIGN_PLUS_RED_ZONE() has already wrapped back to
+ something small and plausible, so neither of them fires and the
+ caller is handed a block it believes is nearly SIZE_MAX bytes long.
+ The documented contract is to return NULL when the pool cannot
+ support @a new_size bytes. */
+ if (new_size > pool->size)
+ return NULL;
+
if (NULL != old)
{ /* Have previously allocated data */
const size_t old_offset = mp_ptr_diff_ (old, pool->memory);
@@ -589,6 +600,15 @@ MHD_pool_reallocate (struct MemoryPool *pool,
ROUND_TO_ALIGN_PLUS_RED_ZONE (old_offset + new_size);
if (! shrinking)
{ /* Grow in-place, check for enough space. */
+ /* Reject a wrapping @a new_size before looking at 'new_apos'.
+ 'old_offset + new_size' can wrap all the way round and land
+ back inside [pool->pos, pool->end], and the two tests below
+ then both pass: the caller is handed a block it believes is
+ nearly SIZE_MAX bytes long. The "allocate a new block" path
+ further down already has an explicit wrap test; this is the
+ same check for the in-place path. */
+ if (new_size > pool->size - old_offset)
+ return NULL; /* Value wrap, or beyond the pool */
if ( (new_apos > pool->end) ||
(new_apos < pool->pos) ) /* Value wrap */
return NULL; /* No space */
@@ -652,7 +672,12 @@ MHD_pool_deallocate (struct MemoryPool *pool,
const size_t block_offset = mp_ptr_diff_ (block, pool->memory);
mhd_assert (mp_ptr_le_ (pool->memory, block));
mhd_assert (block_offset <= pool->size);
- mhd_assert ((block_offset != pool->pos) || (block_size == 0));
+ /* A block allocated "from the end" starts at or after pool->end, a
+ "normal" block starts before it. Those two ranges meet when the
+ pool is exactly full (pool->pos == pool->end), so an end block may
+ legitimately start at pool->pos; only a normal block may not. */
+ mhd_assert ((block_offset >= pool->end) || \
+ (block_offset != pool->pos) || (block_size == 0));
/* Zero-out deallocated region */
if (0 != block_size)
{
@@ -663,9 +688,13 @@ MHD_pool_deallocate (struct MemoryPool *pool,
else
return; /* Zero size, no need to do anything */
#endif /* ! MHD_FAVOR_SMALL_CODE && ! MHD_ASAN_POISON_ACTIVE */
- if (block_offset <= pool->pos)
+ if (block_offset < pool->end)
{
- /* "Normal" block, not allocated "from the end". */
+ /* "Normal" block, not allocated "from the end".
+ The test is against pool->end, not pool->pos: when the pool is
+ exactly full the two are equal, and a block allocated "from the
+ end" then also satisfies 'block_offset <= pool->pos', so it
+ would be mistaken for a normal block and never returned. */
const size_t alg_end =
ROUND_TO_ALIGN_PLUS_RED_ZONE (block_offset + block_size);
mhd_assert (alg_end <= pool->pos);
diff --git a/src/microhttpd/postprocessor.c b/src/microhttpd/postprocessor.c
@@ -1108,21 +1108,35 @@ post_process_multipart (struct MHD_PostProcessor *pp,
"multipart/mixed",
MHD_STATICSTR_LEN_ ("multipart/mixed"))))
{
- pp->nested_boundary = strstr (pp->content_type,
- "boundary=");
- if (NULL == pp->nested_boundary)
+ const char *bnd;
+ char *nested;
+
+ bnd = strstr (pp->content_type,
+ "boundary=");
+ if (NULL == bnd)
{
pp->state = PP_Error;
return MHD_NO;
}
- pp->nested_boundary =
- strdup (&pp->nested_boundary[MHD_STATICSTR_LEN_ ("boundary=")]);
- if (NULL == pp->nested_boundary)
+ /* Note: 'bnd' points into 'pp->content_type', which is released
+ below, so the copy must be taken first. It must also be
+ taken into a local: assigning the 'strstr()' result to
+ 'pp->nested_boundary' first would overwrite -- and leak -- a
+ boundary already owned by this post processor. That happens
+ whenever this state is reached twice without passing through
+ PP_PerformCleanup, i.e. for every additional nested
+ "multipart/mixed" part, and the leaked block is as long as
+ the boundary the client chose. */
+ nested = strdup (&bnd[MHD_STATICSTR_LEN_ ("boundary=")]);
+ if (NULL == nested)
{
/* out of memory */
pp->state = PP_Error;
return MHD_NO;
}
+ if (NULL != pp->nested_boundary)
+ free (pp->nested_boundary);
+ pp->nested_boundary = nested;
/* free old content type, we will need that field
for the content type of the nested elements */
free (pp->content_type);
