commit 313d7b5d744866078bb9db579833b261930251c5
author Ron Eldor <Ron.Eldor@arm.com> Mon Dec 10 14:56:21 2018 +0200
committer Johan Pascal <johan.pascal@belledonne-communications.com> Thu Oct 29 01:14:49 2020 +0100
tree 65312352b831f71b6f4f13dbe77f742e446fe011
parent 75870ec6a7ad82a60a0faceeb4f2aca321e28cec

Add variable validation

1. Check allocation success.
2. Check parameter correctness in the use_srtp extension
in server and client.

Signed-off-by: Johan Pascal <johan.pascal@belledonne-communications.com>

library/ssl_cli.c

diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index a6940f9..fd177ac 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -837,6 +837,10 @@
     if( mki_len != 0 )
     {
         memcpy( p, ssl->dtls_srtp_info.mki_value, mki_len );
+        /*
+         * Increment p to point to the current position.
+         */
+        p += mki_len;
         MBEDTLS_SSL_DEBUG_BUF( 3, "sending mki",  ssl->dtls_srtp_info.mki_value,
                                ssl->dtls_srtp_info.mki_len );
     }
@@ -847,8 +851,9 @@
      *                         + protection profile length (2 bytes)
      *                         + 2 * number of protection profiles
      *                         + srtp_mki vector length(1 byte)
+     *                         + mki value
      */
-    *olen = 2 + 2 + 2 + 2 * ( ssl->conf->dtls_srtp_profile_list_len ) + 1 + mki_len;
+    *olen = p - buf;
 }
 #endif /* MBEDTLS_SSL_DTLS_SRTP */
 
@@ -1848,7 +1853,8 @@
 
     /*
      * Length is 5 and optional mki_value : one protection profile(2 bytes)
-     *                                      + length(2 bytes) and srtp_mki
+     *                                      + length(2 bytes) + mki_len(1 byte)
+     *                                      and optional srtp_mki
      */
     if( ( len != 5 ) && ( len != ( 5 + mki_len ) ) )
         return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
@@ -1862,9 +1868,7 @@
      * one protection profile in server Hello
      */
     if( (  buf[0] != 0 ) || ( buf[1] != 2 ) )
-    {
         return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
-    }
 
     server_protection_profile_value = ( buf[2] << 8 ) | buf[3];
     server_protection = mbedtls_ssl_get_srtp_profile_value( server_protection_profile_value );
@@ -1901,8 +1905,8 @@
      *  that is different than the one the client offered, then the client
      *  MUST abort the handshake and SHOULD send an invalid_parameter alert.
      */
-    if( len > 5  &&
-        ( memcmp( ssl->dtls_srtp_info.mki_value, &buf[5], mki_len ) ) )
+    if( len > 5  && ( buf[4] != mki_len ||
+        ( memcmp( ssl->dtls_srtp_info.mki_value, &buf[5], mki_len ) ) ) )
     {
         mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
                                         MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER );

library/ssl_srv.c

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 38cdd91..56e0cbf 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -785,6 +785,8 @@
     size_t i,j;
     size_t profile_length;
     const mbedtls_ssl_srtp_profile_info *profile_info;
+    /*! 2 bytes for profile length and 1 byte for mki len */
+    const size_t size_of_lengths = 3;
 
     /* If use_srtp is not configured, just ignore the extension */
     if( ssl->conf->dtls_srtp_profile_list == NULL ||
@@ -806,14 +808,24 @@
      * Min length is 5: at least one protection profile(2 bytes)
      *                  and length(2 bytes) + srtp_mki length(1 byte)
      */
-    if( len < 5 )
+    if( len < size_of_lengths + 2 )
+    {
+        mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+                                        MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
         return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
+    }
 
    ssl->dtls_srtp_info.chosen_dtls_srtp_profile = MBEDTLS_SRTP_UNSET_PROFILE;
 
     /* first 2 bytes are protection profile length(in bytes) */
     profile_length = ( buf[0] << 8 ) | buf[1];
 
+    if( profile_length > len - size_of_lengths )
+    {
+        mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+                                        MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
+        return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
+    }
     /*
      * parse the extension list values are defined in
      * http://www.iana.org/assignments/srtp-protection/srtp-protection.xhtml
@@ -846,7 +858,8 @@
         ( len > ( profile_length + 2 ) ) )
     {
         ssl->dtls_srtp_info.mki_len = buf[profile_length + 2];
-        if( ssl->dtls_srtp_info.mki_len > MBEDTLS_DTLS_SRTP_MAX_MKI_LENGTH )
+        if( ssl->dtls_srtp_info.mki_len > MBEDTLS_DTLS_SRTP_MAX_MKI_LENGTH ||
+            ssl->dtls_srtp_info.mki_len + profile_length + size_of_lengths != len )
         {
             mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
                                             MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
@@ -854,7 +867,6 @@
             return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
         }
 
-        ssl->dtls_srtp_info.mki_len = buf[profile_length + 2];
         for( i=0; i < ssl->dtls_srtp_info.mki_len; i++ )
         {
             ssl->dtls_srtp_info.mki_value[i] = buf[profile_length + 2 + 1 + i];

library/ssl_tls.c

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index b15df14..1b4779b 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4807,6 +4807,8 @@
     conf->dtls_srtp_profile_list =
             (mbedtls_ssl_srtp_profile*)mbedtls_calloc(1,
              profiles_number * sizeof( mbedtls_ssl_srtp_profile ) );
+    if( conf->dtls_srtp_profile_list == NULL )
+        return( MBEDTLS_ERR_SSL_ALLOC_FAILED );
 
     for( i=0; i < profiles_number; i++ ) {
         switch( profiles[i] ) {