Merge pull request #6185 from leorosen/tls12_server_null_on_missing_key
ssl_tls12_server: fix potential NULL-dereferencing if local certifica…
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 19b8a41..c45a1b8 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1898,7 +1898,7 @@
* struct {
* uint64 ticket_received;
* uint32 ticket_lifetime;
- * opaque ticket<0..2^16>;
+ * opaque ticket<1..2^16-1>;
* } ClientOnlyData;
*
* struct {
@@ -1915,16 +1915,23 @@
*
*/
#if defined(MBEDTLS_SSL_SESSION_TICKETS)
-static size_t ssl_tls13_session_save( const mbedtls_ssl_session *session,
- unsigned char *buf,
- size_t buf_len )
+MBEDTLS_CHECK_RETURN_CRITICAL
+static int ssl_tls13_session_save( const mbedtls_ssl_session *session,
+ unsigned char *buf,
+ size_t buf_len,
+ size_t *olen )
{
unsigned char *p = buf;
size_t needed = 1 /* endpoint */
+ 2 /* ciphersuite */
+ 4 /* ticket_age_add */
- + 2 /* resumption_key length */
- + session->resumption_key_len; /* resumption_key */
+ + 1 /* ticket_flags */
+ + 1; /* resumption_key length */
+ *olen = 0;
+
+ if( session->resumption_key_len > MBEDTLS_SSL_TLS1_3_TICKET_RESUMPTION_KEY_LEN )
+ return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+ needed += session->resumption_key_len; /* resumption_key */
#if defined(MBEDTLS_HAVE_TIME)
needed += 8; /* start_time or ticket_received */
@@ -1934,13 +1941,19 @@
if( session->endpoint == MBEDTLS_SSL_IS_CLIENT )
{
needed += 4 /* ticket_lifetime */
- + 2 /* ticket_len */
- + session->ticket_len; /* ticket */
+ + 2; /* ticket_len */
+
+ /* Check size_t overflow */
+ if( session->ticket_len > SIZE_MAX - needed )
+ return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+
+ needed += session->ticket_len; /* ticket */
}
#endif /* MBEDTLS_SSL_CLI_C */
+ *olen = needed;
if( needed > buf_len )
- return( needed );
+ return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL );
p[0] = session->endpoint;
MBEDTLS_PUT_UINT16_BE( session->ciphersuite, p, 1 );
@@ -1973,14 +1986,15 @@
MBEDTLS_PUT_UINT16_BE( session->ticket_len, p, 0 );
p += 2;
- if( session->ticket_len > 0 )
+
+ if( session->ticket != NULL && session->ticket_len > 0 )
{
memcpy( p, session->ticket, session->ticket_len );
p += session->ticket_len;
}
}
#endif /* MBEDTLS_SSL_CLI_C */
- return( needed );
+ return( 0 );
}
MBEDTLS_CHECK_RETURN_CRITICAL
@@ -2056,14 +2070,17 @@
}
#else /* MBEDTLS_SSL_SESSION_TICKETS */
-static size_t ssl_tls13_session_save( const mbedtls_ssl_session *session,
- unsigned char *buf,
- size_t buf_len )
+MBEDTLS_CHECK_RETURN_CRITICAL
+static int ssl_tls13_session_save( const mbedtls_ssl_session *session,
+ unsigned char *buf,
+ size_t buf_len,
+ size_t *olen )
{
((void) session);
((void) buf);
((void) buf_len);
- return( 0 );
+ *olen = 0;
+ return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
}
static int ssl_tls13_session_load( const mbedtls_ssl_session *session,
@@ -3007,7 +3024,10 @@
unsigned char *p = buf;
size_t used = 0;
size_t remaining_len;
-
+#if defined(MBEDTLS_SSL_PROTO_TLS1_3)
+ size_t out_len;
+ int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+#endif
if( session == NULL )
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
@@ -3047,7 +3067,10 @@
#if defined(MBEDTLS_SSL_PROTO_TLS1_3)
case MBEDTLS_SSL_VERSION_TLS1_3:
- used += ssl_tls13_session_save( session, p, remaining_len );
+ ret = ssl_tls13_session_save( session, p, remaining_len, &out_len );
+ if( ret != 0 && ret != MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL )
+ return( ret );
+ used += out_len;
break;
#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function
index 33e0bdb..145591d 100644
--- a/tests/suites/test_suite_ssl.function
+++ b/tests/suites/test_suite_ssl.function
@@ -4809,7 +4809,7 @@
original.resumption_key_len ) == 0 );
}
#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C)
- if( endpoint_type == MBEDTLS_SSL_IS_CLIENT)
+ if( endpoint_type == MBEDTLS_SSL_IS_SERVER )
{
TEST_ASSERT( original.start == restored.start );
}