From 05360005e37d66ae5b5e437c2f6ec1dadf13735b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 20 Jun 2021 23:08:19 +0200 Subject: [PATCH 1/2] Refactor file descriptor checks into a common function This will make it easier to change the behavior uniformly. No behavior change. Signed-off-by: Gilles Peskine --- library/net_sockets.c | 54 +++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 8f79b7401a..746ed2a730 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -135,6 +135,26 @@ static int net_prepare( void ) return( 0 ); } +/* + * Return 0 if the file descriptor is valid, an error otherwise. + * If for_select != 0, check whether the file descriptor is within the range + * allowed for fd_set used for the FD_xxx macros and the select() function. + */ +static int check_fd( int fd, int for_select ) +{ + if( fd < 0 ) + return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + + /* A limitation of select() is that it only works with file descriptors + * 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( for_select && fd >= FD_SETSIZE ) + return( MBEDTLS_ERR_NET_POLL_FAILED ); + + return( 0 ); +} + /* * Initialize a context */ @@ -466,15 +486,9 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) int fd = ctx->fd; - if( fd < 0 ) - return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); - - /* A limitation of select() is that it only works with file descriptors - * 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 ); + ret = check_fd( fd, 1 ); + if( ret != 0 ) + return( ret ); #if defined(__has_feature) #if __has_feature(memory_sanitizer) @@ -553,8 +567,9 @@ int mbedtls_net_recv( void *ctx, unsigned char *buf, size_t len ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int fd = ((mbedtls_net_context *) ctx)->fd; - if( fd < 0 ) - return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + ret = check_fd( fd, 0 ); + if( ret != 0 ) + return( ret ); ret = (int) read( fd, buf, len ); @@ -592,15 +607,9 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, fd_set read_fds; int fd = ((mbedtls_net_context *) ctx)->fd; - if( fd < 0 ) - return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); - - /* A limitation of select() is that it only works with file descriptors - * 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 ); + ret = check_fd( fd, 1 ); + if( ret != 0 ) + return( ret ); FD_ZERO( &read_fds ); FD_SET( fd, &read_fds ); @@ -640,8 +649,9 @@ int mbedtls_net_send( void *ctx, const unsigned char *buf, size_t len ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int fd = ((mbedtls_net_context *) ctx)->fd; - if( fd < 0 ) - return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + ret = check_fd( fd, 0 ); + if( ret != 0 ) + return( ret ); ret = (int) write( fd, buf, len ); From a5dd7bded8a0fb84043db1ef28b3577d173a54a1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 20 Jun 2021 22:01:36 +0200 Subject: [PATCH 2/2] Fix fd range for select on Windows Fix mbedtls_net_poll() and mbedtls_net_recv_timeout() often failing with MBEDTLS_ERR_NET_POLL_FAILED on Windows: they were testing that the file descriptor is in range for fd_set, but on Windows socket descriptors are not limited to a small range. Fixes #4465. Signed-off-by: Gilles Peskine --- ChangeLog.d/winsock.txt | 4 ++++ library/net_sockets.c | 5 +++++ 2 files changed, 9 insertions(+) create mode 100644 ChangeLog.d/winsock.txt diff --git a/ChangeLog.d/winsock.txt b/ChangeLog.d/winsock.txt new file mode 100644 index 0000000000..0b42e691c2 --- /dev/null +++ b/ChangeLog.d/winsock.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix mbedtls_net_poll() and mbedtls_net_recv_timeout() often failing with + MBEDTLS_ERR_NET_POLL_FAILED on Windows. Fixes #4465. + diff --git a/library/net_sockets.c b/library/net_sockets.c index 746ed2a730..5fbe1f764a 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -145,12 +145,17 @@ static int check_fd( int fd, int for_select ) if( fd < 0 ) return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); +#if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(EFIX64) && \ + !defined(EFI32) + (void) for_select; +#else /* A limitation of select() is that it only works with file descriptors * 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( for_select && fd >= FD_SETSIZE ) return( MBEDTLS_ERR_NET_POLL_FAILED ); +#endif return( 0 ); }