From 33d816aff99020b57b54961df60ca0e843a1178f 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_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 | 3 +++ library/net_sockets.c | 7 +++++++ 2 files changed, 10 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..23b11bb59 --- /dev/null +++ b/ChangeLog.d/net_poll-fd_setsize.txt @@ -0,0 +1,3 @@ +Security + * Fix a stack buffer overflow with 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 2876f8fdd..e19d84a29 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -535,6 +535,13 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, size_t len, 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_RECV_FAILED ); + FD_ZERO( &read_fds ); FD_SET( fd, &read_fds ); From 51917a82e83104ddfd58b8363e85f01d81d836b3 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_recv_timeout Signed-off-by: Gilles Peskine --- include/mbedtls/net_sockets.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index 0d61547d0..67fe1e9f2 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -219,6 +219,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 f02eeb87620acb20d93e2ccd5189f499369de7f4 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 | 2 +- library/net_sockets.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index 67fe1e9f2..bca35839f 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -221,7 +221,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 e19d84a29..fa27603c1 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -536,9 +536,9 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, size_t len, 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_RECV_FAILED ); From 51f5d31635064fe984cb5f57a725af2d0ad4fdc1 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 bca35839f..5f2b1c502 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -130,6 +130,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 * @@ -149,6 +150,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 @@ -229,10 +232,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