diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6f9203daa..cc70510cb 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -9959,11 +9959,14 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, } /* - * Unserialise session, see ssl_save_session() + * Unserialise session, see mbedtls_ssl_session_save(). + * + * This internal version is wrapped by a public function that cleans up in + * case of error. */ -int mbedtls_ssl_session_load( mbedtls_ssl_session *session, - const unsigned char *buf, - size_t len ) +static int ssl_session_load( mbedtls_ssl_session *session, + const unsigned char *buf, + size_t len ) { const unsigned char *p = buf; const unsigned char * const end = buf + len; @@ -10091,6 +10094,21 @@ int mbedtls_ssl_session_load( mbedtls_ssl_session *session, return( 0 ); } +/* + * Unserialise session: public wrapper for error cleaning + */ +int mbedtls_ssl_session_load( mbedtls_ssl_session *session, + const unsigned char *buf, + size_t len ) +{ + int ret = ssl_session_load( session, buf, len ); + + if( ret != 0 ) + mbedtls_ssl_session_free( session ); + + return( ret ); +} + /* * Perform a single step of the SSL handshake */ diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index b166a9a93..d41fcd01d 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -8798,3 +8798,26 @@ ssl_serialise_session_save_buf_size:42:"data_files/server5.crt" Session serialisation, save buffer size: large ticket, cert depends_on:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C:MBEDTLS_X509_USE_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C ssl_serialise_session_save_buf_size:1023:"data_files/server5.crt" + +Session serialisation, load buffer size: no ticket, no cert +ssl_serialise_session_load_buf_size:0:"" + +Session serialisation, load buffer size: small ticket, no cert +depends_on:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C +ssl_serialise_session_load_buf_size:42:"" + +Session serialisation, load buffer size: large ticket, no cert +depends_on:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C +ssl_serialise_session_load_buf_size:1023:"" + +Session serialisation, load buffer size: no ticket, cert +depends_on:MBEDTLS_X509_USE_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C +ssl_serialise_session_load_buf_size:0:"data_files/server5.crt" + +Session serialisation, load buffer size: small ticket, cert +depends_on:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C:MBEDTLS_X509_USE_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C +ssl_serialise_session_load_buf_size:42:"data_files/server5.crt" + +Session serialisation, load buffer size: large ticket, cert +depends_on:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C:MBEDTLS_X509_USE_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C +ssl_serialise_session_load_buf_size:1023:"data_files/server5.crt" diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 61ff41608..8a184d0f8 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -748,3 +748,45 @@ exit: mbedtls_free( buf ); } /* END_CASE */ + +/* BEGIN_CASE */ +void ssl_serialise_session_load_buf_size( int ticket_len, char *crt_file ) +{ + mbedtls_ssl_session session; + unsigned char *good_buf = NULL, *bad_buf = NULL; + size_t good_len, bad_len; + + /* + * Test that session_load() fails cleanly on small buffers + */ + + mbedtls_ssl_session_init( &session ); + + /* Prepare serialised session data */ + ssl_populate_session( &session, ticket_len, crt_file ); + TEST_ASSERT( mbedtls_ssl_session_save( &session, NULL, 0, &good_len ) + == MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + TEST_ASSERT( ( good_buf = mbedtls_calloc( 1, good_len ) ) != NULL ); + TEST_ASSERT( mbedtls_ssl_session_save( &session, good_buf, good_len, + &good_len ) == 0 ); + mbedtls_ssl_session_free( &session ); + + /* Try all possible bad lengths */ + for( bad_len = 0; bad_len < good_len; bad_len++ ) + { + /* Allocate exact size so that asan/valgrind can detect any overread */ + mbedtls_free( bad_buf ); + bad_buf = mbedtls_calloc( 1, bad_len ? bad_len : 1 ); + TEST_ASSERT( bad_buf != NULL ); + memcpy( bad_buf, good_buf, bad_len ); + + TEST_ASSERT( mbedtls_ssl_session_load( &session, bad_buf, bad_len ) + == MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + +exit: + mbedtls_ssl_session_free( &session ); + mbedtls_free( good_buf ); + mbedtls_free( bad_buf ); +} +/* END_CASE */