From a2fce21ae5b5bd74c66e305f864ff5875cf9df81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 15 Apr 2015 19:09:03 +0200 Subject: [PATCH] Fix potential NULL dereference on bad usage --- ChangeLog | 3 ++ library/ssl_tls.c | 71 ++++++++++++++++++++++++++++------------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index edcd42dd3..39ae43f82 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,9 @@ Features errors on use of deprecated functions. Bugfix + * Fix potential NULL pointer dereference (not trigerrable remotely) when + ssl_write() is called before the handshake is finished (introduced in + 1.3.10) (first reported by Martin Blumenstingl). * Fix bug in pk_parse_key() that caused some valid private EC keys to be rejected. * Fix bug in Via Padlock support (found by Nikos Mavrogiannopoulos). diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 515b90355..d2e0c52bc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4754,37 +4754,16 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } /* - * Send application data to be encrypted by the SSL layer + * Send application data to be encrypted by the SSL layer, + * taking care of max fragment length and buffer size */ -#if defined(POLARSSL_SSL_CBC_RECORD_SPLITTING) -static int ssl_write_real( ssl_context *ssl, const unsigned char *buf, size_t len ) -#else -int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) -#endif +static int ssl_write_real( ssl_context *ssl, + const unsigned char *buf, size_t len ) { int ret; size_t n; unsigned int max_len = SSL_MAX_CONTENT_LEN; - SSL_DEBUG_MSG( 2, ( "=> write" ) ); - -#if defined(POLARSSL_SSL_RENEGOTIATION) - if( ( ret = ssl_check_ctr_renegotiate( ssl ) ) != 0 ) - { - SSL_DEBUG_RET( 1, "ssl_check_ctr_renegotiate", ret ); - return( ret ); - } -#endif - - if( ssl->state != SSL_HANDSHAKE_OVER ) - { - if( ( ret = ssl_handshake( ssl ) ) != 0 ) - { - SSL_DEBUG_RET( 1, "ssl_handshake", ret ); - return( ret ); - } - } - #if defined(POLARSSL_SSL_MAX_FRAGMENT_LENGTH) /* * Assume mfl_code is correct since it was checked when set @@ -4824,8 +4803,6 @@ int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) } } - SSL_DEBUG_MSG( 2, ( "<= write" ) ); - return( (int) n ); } @@ -4837,7 +4814,8 @@ int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) * remember wether we already did the split or not. */ #if defined(POLARSSL_SSL_CBC_RECORD_SPLITTING) -int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) +static int ssl_write_split( ssl_context *ssl, + const unsigned char *buf, size_t len ) { int ret; @@ -4865,6 +4843,43 @@ int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) } #endif /* POLARSSL_SSL_CBC_RECORD_SPLITTING */ +/* + * Write application data (public-facing wrapper) + */ +int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) +{ + int ret; + + SSL_DEBUG_MSG( 2, ( "=> write" ) ); + +#if defined(POLARSSL_SSL_RENEGOTIATION) + if( ( ret = ssl_check_ctr_renegotiate( ssl ) ) != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_check_ctr_renegotiate", ret ); + return( ret ); + } +#endif + + if( ssl->state != SSL_HANDSHAKE_OVER ) + { + if( ( ret = ssl_handshake( ssl ) ) != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_handshake", ret ); + return( ret ); + } + } + +#if defined(POLARSSL_SSL_CBC_RECORD_SPLITTING) + ret = ssl_write_split( ssl, buf, len ); +#else + ret = ssl_write_real( ssl, buf, len ); +#endif + + SSL_DEBUG_MSG( 2, ( "<= write" ) ); + + return( ret ); +} + /* * Notify the peer that the connection is being closed */