From f604240b1bf53dda98932b9d73b7afa36720cb7e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Feb 2021 19:49:44 +0100 Subject: [PATCH 1/4] Fix stack buffer overflow in net functions with large file descriptor Fix a stack buffer overflow with mbedtls_net_poll() and mbedtls_net_recv_timeout() when given a file descriptor that is beyond FD_SETSIZE. The bug was due to not checking that the file descriptor is within the range of an fd_set object. Fix #4169 Signed-off-by: Gilles Peskine --- ChangeLog.d/net_poll-fd_setsize.txt | 4 ++++ library/net_sockets.c | 14 ++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 ChangeLog.d/net_poll-fd_setsize.txt diff --git a/ChangeLog.d/net_poll-fd_setsize.txt b/ChangeLog.d/net_poll-fd_setsize.txt new file mode 100644 index 000000000..e4db8c7e3 --- /dev/null +++ b/ChangeLog.d/net_poll-fd_setsize.txt @@ -0,0 +1,4 @@ +Security + * Fix a stack buffer overflow with mbedtls_net_poll() and + mbedtls_net_recv_timeout() when given a file descriptor that is + beyond FD_SETSIZE. Reported by FigBug in #4169. diff --git a/library/net_sockets.c b/library/net_sockets.c index 113040826..b2f76a002 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -496,6 +496,13 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) if( fd < 0 ) return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + /* A limitation of select() is that it only works with file descriptors + * up to FD_SETSIZE. This is a limitation of the fd_set type. Error out + * early, because attempting to call FD_SET on a large file descriptor + * is a buffer overflow on typical platforms. */ + if( fd >= FD_SETSIZE ) + return( MBEDTLS_ERR_NET_POLL_FAILED ); + #if defined(__has_feature) #if __has_feature(memory_sanitizer) /* Ensure that memory sanitizers consider read_fds and write_fds as @@ -615,6 +622,13 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, if( fd < 0 ) return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + /* A limitation of select() is that it only works with file descriptors + * up to FD_SETSIZE. This is a limitation of the fd_set type. Error out + * early, because attempting to call FD_SET on a large file descriptor + * is a buffer overflow on typical platforms. */ + if( fd >= FD_SETSIZE ) + return( MBEDTLS_ERR_NET_POLL_FAILED ); + FD_ZERO( &read_fds ); FD_SET( fd, &read_fds ); From 58ec37891289e02093607d890ba954c02218f4f6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Feb 2021 19:51:23 +0100 Subject: [PATCH 2/4] Document FD_SETSIZE limitation for mbedtls_net_{poll,recv_timeout} Signed-off-by: Gilles Peskine --- include/mbedtls/net_sockets.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index 00fea7db1..f89f73d1a 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -182,6 +182,10 @@ int mbedtls_net_accept( mbedtls_net_context *bind_ctx, /** * \brief Check and wait for the context to be ready for read/write * + * \note The current implementation of this function uses + * select() and returns an error if the file descriptor + * is beyond \c FD_SETSIZE. + * * \param ctx Socket to check * \param rw Bitflag composed of MBEDTLS_NET_POLL_READ and * MBEDTLS_NET_POLL_WRITE specifying the events @@ -263,6 +267,10 @@ int mbedtls_net_send( void *ctx, const unsigned char *buf, size_t len ); * 'timeout' seconds. If no error occurs, the actual amount * read is returned. * + * \note The current implementation of this function uses + * select() and returns an error if the file descriptor + * is beyond \c FD_SETSIZE. + * * \param ctx Socket * \param buf The buffer to write to * \param len Maximum length of the buffer From 121d7c7c14da6a0b6188d62f52a9d8548061057c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 1 Mar 2021 11:39:21 +0100 Subject: [PATCH 3/4] Fix sloppy wording around stricly less-than vs less or equal Signed-off-by: Gilles Peskine --- include/mbedtls/net_sockets.h | 4 ++-- library/net_sockets.c | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index f89f73d1a..d8188494e 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -184,7 +184,7 @@ int mbedtls_net_accept( mbedtls_net_context *bind_ctx, * * \note The current implementation of this function uses * select() and returns an error if the file descriptor - * is beyond \c FD_SETSIZE. + * is \c FD_SETSIZE or greater. * * \param ctx Socket to check * \param rw Bitflag composed of MBEDTLS_NET_POLL_READ and @@ -269,7 +269,7 @@ int mbedtls_net_send( void *ctx, const unsigned char *buf, size_t len ); * * \note The current implementation of this function uses * select() and returns an error if the file descriptor - * is beyond \c FD_SETSIZE. + * is \c FD_SETSIZE or greater. * * \param ctx Socket * \param buf The buffer to write to diff --git a/library/net_sockets.c b/library/net_sockets.c index b2f76a002..671115f15 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -497,9 +497,9 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); /* A limitation of select() is that it only works with file descriptors - * up to FD_SETSIZE. This is a limitation of the fd_set type. Error out - * early, because attempting to call FD_SET on a large file descriptor - * is a buffer overflow on typical platforms. */ + * that are strictly less than FD_SETSIZE. This is a limitation of the + * fd_set type. Error out early, because attempting to call FD_SET on a + * large file descriptor is a buffer overflow on typical platforms. */ if( fd >= FD_SETSIZE ) return( MBEDTLS_ERR_NET_POLL_FAILED ); @@ -623,9 +623,9 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); /* A limitation of select() is that it only works with file descriptors - * up to FD_SETSIZE. This is a limitation of the fd_set type. Error out - * early, because attempting to call FD_SET on a large file descriptor - * is a buffer overflow on typical platforms. */ + * that are strictly less than FD_SETSIZE. This is a limitation of the + * fd_set type. Error out early, because attempting to call FD_SET on a + * large file descriptor is a buffer overflow on typical platforms. */ if( fd >= FD_SETSIZE ) return( MBEDTLS_ERR_NET_POLL_FAILED ); From 388a9d3a8bd14a37354ee3611e58d6d92c40bf14 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 3 Mar 2021 12:25:06 +0100 Subject: [PATCH 4/4] Update error codes listed in the net_sockets documentation Signed-off-by: Gilles Peskine --- include/mbedtls/net_sockets.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index d8188494e..c6e1a0270 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -151,6 +151,7 @@ int mbedtls_net_connect( mbedtls_net_context *ctx, const char *host, const char * * \return 0 if successful, or one of: * MBEDTLS_ERR_NET_SOCKET_FAILED, + * MBEDTLS_ERR_NET_UNKNOWN_HOST, * MBEDTLS_ERR_NET_BIND_FAILED, * MBEDTLS_ERR_NET_LISTEN_FAILED * @@ -170,6 +171,8 @@ int mbedtls_net_bind( mbedtls_net_context *ctx, const char *bind_ip, const char * can be NULL if client_ip is null * * \return 0 if successful, or + * MBEDTLS_ERR_NET_SOCKET_FAILED, + * MBEDTLS_ERR_NET_BIND_FAILED, * MBEDTLS_ERR_NET_ACCEPT_FAILED, or * MBEDTLS_ERR_NET_BUFFER_TOO_SMALL if buf_size is too small, * MBEDTLS_ERR_SSL_WANT_READ if bind_fd was set to @@ -277,10 +280,11 @@ int mbedtls_net_send( void *ctx, const unsigned char *buf, size_t len ); * \param timeout Maximum number of milliseconds to wait for data * 0 means no timeout (wait forever) * - * \return the number of bytes received, - * or a non-zero error code: - * MBEDTLS_ERR_SSL_TIMEOUT if the operation timed out, + * \return The number of bytes received if successful. + * MBEDTLS_ERR_SSL_TIMEOUT if the operation timed out. * MBEDTLS_ERR_SSL_WANT_READ if interrupted by a signal. + * Another negative error code (MBEDTLS_ERR_NET_xxx) + * for other failures. * * \note This function will block (until data becomes available or * timeout is reached) even if the socket is set to