From 0761733c1b098e4f2d2fc815181dc916bb6cb0b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 24 Jun 2015 23:00:03 +0200 Subject: [PATCH] Fix potential NULL dereference We document that either of recv or recv_timeout may be NULL, but for TLS we always used recv... Thanks Coverity for catching that. (Not remotely trigerrable: local configuration.) Also made me notice net_recv_timeout didn't do its job properly. --- include/mbedtls/net.h | 1 + include/mbedtls/ssl.h | 2 +- library/net.c | 2 +- library/ssl_tls.c | 20 ++++++++++++++++---- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/net.h b/include/mbedtls/net.h index ed2d5d322..92f94c0ba 100644 --- a/include/mbedtls/net.h +++ b/include/mbedtls/net.h @@ -178,6 +178,7 @@ int mbedtls_net_send( void *ctx, const unsigned char *buf, size_t len ); * \param buf The buffer to write to * \param len Maximum length of the buffer * \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: diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d75e93ab4..b7e361a72 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -982,7 +982,7 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, * \param f_recv read callback * \param f_recv_timeout blocking read callback with timeout. * The last argument is the timeout in milliseconds, - * 0 means no timeout + * 0 means no timeout (block forever until a message comes) * * \note One of f_recv or f_recv_timeout can be NULL, in which case * the other is used. If both are non-NULL, f_recv_timeout is diff --git a/library/net.c b/library/net.c index f284153a9..bfa7072d1 100644 --- a/library/net.c +++ b/library/net.c @@ -444,7 +444,7 @@ int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, size_t len, tv.tv_sec = timeout / 1000; tv.tv_usec = ( timeout % 1000 ) * 1000; - ret = select( fd + 1, &read_fds, NULL, NULL, &tv ); + ret = select( fd + 1, &read_fds, NULL, NULL, timeout == 0 ? NULL : &tv ); /* Zero fds ready means we timed out */ if( ret == 0 ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 222536db7..529cbeb4c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2293,7 +2293,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "f_recv_timeout: %u ms", timeout ) ); - if( ssl->f_recv_timeout != NULL && timeout != 0 ) + if( ssl->f_recv_timeout != NULL ) ret = ssl->f_recv_timeout( ssl->p_bio, ssl->in_hdr, len, timeout ); else @@ -2359,11 +2359,23 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) if( ssl_check_timer( ssl ) != 0 ) ret = MBEDTLS_ERR_SSL_TIMEOUT; else - ret = ssl->f_recv( ssl->p_bio, ssl->in_hdr + ssl->in_left, len ); + { + if( ssl->f_recv_timeout != NULL ) + { + ret = ssl->f_recv_timeout( ssl->p_bio, + ssl->in_hdr + ssl->in_left, len, + ssl->conf->read_timeout ); + } + else + { + ret = ssl->f_recv( ssl->p_bio, + ssl->in_hdr + ssl->in_left, len ); + } + } MBEDTLS_SSL_DEBUG_MSG( 2, ( "in_left: %d, nb_want: %d", - ssl->in_left, nb_want ) ); - MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_recv", ret ); + ssl->in_left, nb_want ) ); + MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_recv(_timeout)", ret ); if( ret == 0 ) return( MBEDTLS_ERR_SSL_CONN_EOF );