commit ea74a89d61005de46c4634e7d6f25d220ae0253b
parent d48ada44c5edffd61bf47074beb94fb819a0fd55
Author: Evgeny Grin (Karlson2k) <k2k@drgrin.dev>
Date: Sat, 6 Sep 2025 23:07:16 +0200
Fixed possible out-of-buffer use when application provides incorrect addr size
Diffstat:
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/src/mhd2/daemon_add_conn.c b/src/mhd2/daemon_add_conn.c
@@ -56,6 +56,7 @@
#include "mhd_assert.h"
#include "sys_bool_type.h"
#include "sys_base_types.h"
+#include "sys_offsetof.h"
#include "sys_sockets_types.h"
#include "sys_sockets_headers.h"
#include "sys_ip_headers.h"
@@ -195,7 +196,7 @@ MHD_FN_PAR_NONNULL_ (1)
MHD_FN_PAR_NONNULL_ (9) MHD_FN_PAR_OUT_ (9) enum MHD_StatusCode
new_connection_prepare_ (struct MHD_Daemon *restrict daemon,
MHD_Socket client_socket,
- const struct sockaddr_storage *restrict addr,
+ const struct sockaddr *restrict addr,
size_t addrlen,
bool external_add,
bool non_blck,
@@ -207,6 +208,8 @@ new_connection_prepare_ (struct MHD_Daemon *restrict daemon,
enum MHD_StatusCode ret;
size_t tls_data_size;
+ mhd_assert ((0 == addrlen) || (NULL != addr));
+
ret = MHD_SC_OK;
*conn_out = NULL;
@@ -271,9 +274,14 @@ new_connection_prepare_ (struct MHD_Daemon *restrict daemon,
ret = MHD_SC_CONNECTION_MEM_ALLOC_FAILURE;
}
else
+ {
memcpy (c->sk.addr.data,
addr,
addrlen);
+#ifdef HAVE_STRUCT_SOCKADDR_SA_LEN
+ ((struct sockaddr*) c->sk.addr.data)->sa_len = addrlen; /* Force correct value */
+#endif /* HAVE_STRUCT_SOCKADDR_SA_LEN */
+ }
}
else
c->sk.addr.data = NULL;
@@ -561,7 +569,7 @@ new_connection_process_ (struct MHD_Daemon *restrict daemon,
static enum MHD_StatusCode
internal_add_connection (struct MHD_Daemon *daemon,
MHD_Socket client_socket,
- const struct sockaddr_storage *addr,
+ const struct sockaddr *addr,
size_t addrlen,
bool external_add,
bool non_blck,
@@ -685,7 +693,6 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
{
bool sk_nonbl;
bool sk_spipe_supprs;
- struct sockaddr_storage addrstorage;
// TODO: global daemon lock for external events
(void) connection_cntx; // FIXME: is it really needed? Where it is used?
@@ -702,6 +709,15 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
if (0 != addrlen)
{
+ if (addrlen < (sizeof(addr->sa_family)
+ + offsetof (struct sockaddr, sa_family)))
+ {
+ mhd_LOG_MSG (daemon, MHD_SC_CONFIGURATION_WRONG_SA_SIZE, \
+ "MHD_add_connection() has been called with " \
+ "incorrect 'addrlen' value.");
+ (void) mhd_socket_close (client_socket);
+ return MHD_SC_CONFIGURATION_WRONG_SA_SIZE;
+ }
if (AF_INET == addr->sa_family)
{
if (sizeof(struct sockaddr_in) > addrlen)
@@ -726,7 +742,7 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
#endif /* HAVE_STRUCT_SOCKADDR_SA_LEN */
}
#ifdef HAVE_INET6
- if (AF_INET6 == addr->sa_family)
+ else if (AF_INET6 == addr->sa_family)
{
if (sizeof(struct sockaddr_in6) > addrlen)
{
@@ -750,6 +766,15 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
#endif /* HAVE_STRUCT_SOCKADDR_SA_LEN */
}
#ifdef HAVE_STRUCT_SOCKADDR_SA_LEN
+ else if (addrlen < (sizeof(addr->sa_len)
+ + offsetof (struct sockaddr, sa_len)))
+ {
+ mhd_LOG_MSG (daemon, MHD_SC_CONFIGURATION_WRONG_SA_SIZE, \
+ "MHD_add_connection() has been called with " \
+ "incorrect 'addrlen' value.");
+ (void) mhd_socket_close (client_socket);
+ return MHD_SC_CONFIGURATION_WRONG_SA_SIZE;
+ }
if ((0 != addr->sa_len) &&
(addrlen > (size_t) addr->sa_len))
addrlen = (size_t) addr->sa_len; /* Use safest value */
@@ -807,9 +832,6 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
"Failed to set noninheritable mode on new client socket.");
}
- /* Copy to sockaddr_storage structure to avoid alignment problems */
- if (0 < addrlen)
- memcpy (&addrstorage, addr, addrlen);
#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
addrstorage.ss_len = addrlen; /* Force set the right length */
#endif /* HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN */
@@ -845,7 +867,7 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
continue;
return internal_add_connection (worker,
client_socket,
- &addrstorage,
+ addr,
addrlen,
true,
sk_nonbl,
@@ -861,7 +883,7 @@ MHD_daemon_add_connection (struct MHD_Daemon *MHD_RESTRICT daemon,
return internal_add_connection (daemon,
client_socket,
- &addrstorage,
+ addr,
addrlen,
true,
sk_nonbl,
@@ -1081,6 +1103,7 @@ mhd_daemon_accept_connection (struct MHD_Daemon *restrict daemon)
#endif /* mhd_socket_nosignal */
return (MHD_SC_OK == internal_add_connection (daemon,
s,
+ (const struct sockaddr *)
&addrstorage,
(size_t) addrlen,
false,