Skip to content

Commit

Permalink
Fix unsafe bounds checks in ssl_load_session()
Browse files Browse the repository at this point in the history
This commit replaces multiple bounds checks of the form

     `if( ptr + offset > end )`

by

     `if( offset > end - ptr )`.

The former is unsafe as the pointer arithmetic `ptr + offset` can
overflow in case of a large value of `offset` paired with a value
of `ptr` close to the (virtual) address boundary.

The latter bounds check, in turn, is always safe if `offset` is a
signed integral value, even if `ptr` happens to be larger than
`end` (which should never happen, but it's better to be prepared
just in case).

Concretely, ssl_load_session() contains the bounds check

     `if( p + cert_len > end )`

where `cert_len` is a 24-bit value of type `size_t`. This
check is accordingly replaced by  `if( (int) cert_len > end - p )`;
the explicit cast `(int) cert_len` is safe because `int` is assumed
to be 32-bit wide and paddingless, hence capable of holding 24-bit
values.

Fixes Mbed-TLS#659 reported by Guido Vranken.
  • Loading branch information
Hanno Becker committed Oct 24, 2018
1 parent 9543373 commit aa7503d
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions library/ssl_ticket.c
Expand Up @@ -215,14 +215,14 @@ static int ssl_load_session( mbedtls_ssl_session *session,
size_t cert_len;
#endif /* MBEDTLS_X509_CRT_PARSE_C */

if( p + sizeof( mbedtls_ssl_session ) > end )
if( (int) sizeof( mbedtls_ssl_session ) > end - p )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );

memcpy( session, p, sizeof( mbedtls_ssl_session ) );
p += sizeof( mbedtls_ssl_session );

#if defined(MBEDTLS_X509_CRT_PARSE_C)
if( p + 3 > end )
if( 3 > end - p )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );

cert_len = ( p[0] << 16 ) | ( p[1] << 8 ) | p[2];
Expand All @@ -236,7 +236,9 @@ static int ssl_load_session( mbedtls_ssl_session *session,
{
int ret;

if( p + cert_len > end )
/* Explicit cast of cert_len is safe because it's < 2^24
* and we assume at least 32-bit integers. */
if( (int) cert_len > end - p )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );

session->peer_cert = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
Expand Down

0 comments on commit aa7503d

Please sign in to comment.