diff --git a/ChangeLog b/ChangeLog index fa34de7e9..4cee23e4d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -30,6 +30,9 @@ Security Reported by Marco Macchetti, Kudelski Group. * Wipe stack buffer temporarily holding EC private exponent after keypair generation. + * Fix a potential heap buffer overread in ALPN extension parsing + (server-side). Could result in application crash, but only if an ALPN + name larger than 16 bytes had been configured on the server. Features * Allow comments in test data files. diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 90995c08d..2cdf2277c 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -791,25 +791,33 @@ static int ssl_parse_alpn_ext( ssl_context *ssl, return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); /* - * Use our order of preference + * Validate peer's list (lengths) */ start = buf + 2; end = buf + len; + for( theirs = start; theirs != end; theirs += cur_len ) + { + cur_len = *theirs++; + + /* Current identifier must fit in list */ + if( cur_len > (size_t)( end - theirs ) ) + return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); + + /* Empty strings MUST NOT be included */ + if( cur_len == 0 ) + return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); + } + + /* + * Use our order of preference + */ for( ours = ssl->alpn_list; *ours != NULL; ours++ ) { ours_len = strlen( *ours ); for( theirs = start; theirs != end; theirs += cur_len ) { - /* If the list is well formed, we should get equality first */ - if( theirs > end ) - return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); - cur_len = *theirs++; - /* Empty strings MUST NOT be included */ - if( cur_len == 0 ) - return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO ); - if( cur_len == ours_len && memcmp( theirs, *ours, cur_len ) == 0 ) {