Fix for SRTP Memory Leak

CVE-2014-3513

This issue was reported to OpenSSL on 26th September 2014, based on an origi
issue and patch developed by the LibreSSL project. Further analysis of the i
was performed by the OpenSSL team.

The fix was developed by the OpenSSL team.

Reviewed-by: Tim Hudson <tjh@openssl.org>

diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index ab9c41922c46ad07c68b878d25158e806d9d3f74..535539ba3b50c0d211ec83700db0f07e29a6c288 100644 (file)
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -168,25 +168,6 @@ static int find_profile_by_name(char *profile_name,
        return 1;
        }
 
-static int find_profile_by_num(unsigned profile_num,
-                              SRTP_PROTECTION_PROFILE **pptr)
-       {
-       SRTP_PROTECTION_PROFILE *p;
-
-       p=srtp_known_profiles;
-       while(p->name)
-               {
-               if(p->id == profile_num)
-                       {
-                       *pptr=p;
-                       return 0;
-                       }
-               p++;
-               }
-
-       return 1;
-       }
-
 static int ssl_ctx_make_profiles(const char *profiles_string,STACK_OF(SRTP_PROTECTION_PROFILE) **out)
        {
        STACK_OF(SRTP_PROTECTION_PROFILE) *profiles;
@@ -209,11 +190,19 @@ static int ssl_ctx_make_profiles(const char *profiles_string,STACK_OF(SRTP_PROTE
                if(!find_profile_by_name(ptr,&p,
                                         col ? col-ptr : (int)strlen(ptr)))
                        {
+                       if (sk_SRTP_PROTECTION_PROFILE_find(profiles,p) >= 0)
+                               {
+                               SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
+                               sk_SRTP_PROTECTION_PROFILE_free(profiles);
+                               return 1;
+                               }
+
                        sk_SRTP_PROTECTION_PROFILE_push(profiles,p);
                        }
                else
                        {
                        SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,SSL_R_SRTP_UNKNOWN_PROTECTION_PROFILE);
+                       sk_SRTP_PROTECTION_PROFILE_free(profiles);
                        return 1;
                        }
 
@@ -305,13 +294,12 @@ int ssl_add_clienthello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int max
 
 int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al)
        {
-       SRTP_PROTECTION_PROFILE *cprof,*sprof;
-       STACK_OF(SRTP_PROTECTION_PROFILE) *clnt=0,*srvr;
+       SRTP_PROTECTION_PROFILE *sprof;
+       STACK_OF(SRTP_PROTECTION_PROFILE) *srvr;
         int ct;
         int mki_len;
-       int i,j;
-       int id;
-       int ret;
+       int i, srtp_pref;
+       unsigned int id;
 
          /* Length value + the MKI length */
         if(len < 3)
@@ -341,22 +329,32 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
                return 1;
                }
 
+       srvr=SSL_get_srtp_profiles(s);
+       s->srtp_profile = NULL;
+       /* Search all profiles for a match initially */
+       srtp_pref = sk_SRTP_PROTECTION_PROFILE_num(srvr);
         
-       clnt=sk_SRTP_PROTECTION_PROFILE_new_null();
-
        while(ct)
                {
                n2s(d,id);
                ct-=2;
                 len-=2;
 
-               if(!find_profile_by_num(id,&cprof))
+               /*
+                * Only look for match in profiles of higher preference than
+                * current match.
+                * If no profiles have been have been configured then this
+                * does nothing.
+                */
+               for (i = 0; i < srtp_pref; i++)
                        {
-                       sk_SRTP_PROTECTION_PROFILE_push(clnt,cprof);
-                       }
-               else
-                       {
-                       ; /* Ignore */
+                       sprof = sk_SRTP_PROTECTION_PROFILE_value(srvr, i);
+                       if (sprof->id == id)
+                               {
+                               s->srtp_profile = sprof;
+                               srtp_pref = i;
+                               break;
+                               }
                        }
                }
 
@@ -371,36 +369,7 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
                return 1;
                }
 
-       srvr=SSL_get_srtp_profiles(s);
-
-       /* Pick our most preferred profile. If no profiles have been
-        configured then the outer loop doesn't run 
-        (sk_SRTP_PROTECTION_PROFILE_num() = -1)
-        and so we just return without doing anything */
-       for(i=0;i<sk_SRTP_PROTECTION_PROFILE_num(srvr);i++)
-               {
-               sprof=sk_SRTP_PROTECTION_PROFILE_value(srvr,i);
-
-               for(j=0;j<sk_SRTP_PROTECTION_PROFILE_num(clnt);j++)
-                       {
-                       cprof=sk_SRTP_PROTECTION_PROFILE_value(clnt,j);
-            
-                       if(cprof->id==sprof->id)
-                               {
-                               s->srtp_profile=sprof;
-                               *al=0;
-                               ret=0;
-                               goto done;
-                               }
-                       }
-               }
-
-       ret=0;
-    
-done:
-       if(clnt) sk_SRTP_PROTECTION_PROFILE_free(clnt);
-
-       return ret;
+       return 0;
        }
 
 int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int maxlen)

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 022a4fb289e36f8a5b941110a534b9ac73c13534..12ee3c9a104d9bd925c8530c080e4195e6e02d78 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -643,7 +643,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c
 #endif
 
 #ifndef OPENSSL_NO_SRTP
-        if(SSL_get_srtp_profiles(s))
+       if(SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s))
                 {
                 int el;
 
@@ -806,7 +806,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c
 #endif
 
 #ifndef OPENSSL_NO_SRTP
-        if(s->srtp_profile)
+       if(SSL_IS_DTLS(s) && s->srtp_profile)
                 {
                 int el;
 
@@ -1444,7 +1444,8 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in
 
                /* session ticket processed earlier */
 #ifndef OPENSSL_NO_SRTP
-               else if (type == TLSEXT_TYPE_use_srtp)
+               else if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)
+                        && type == TLSEXT_TYPE_use_srtp)
                        {
                        if(ssl_parse_clienthello_use_srtp_ext(s, data, size,
                                                              al))
@@ -1698,7 +1699,7 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in
                        }
 #endif
 #ifndef OPENSSL_NO_SRTP
-               else if (type == TLSEXT_TYPE_use_srtp)
+               else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_use_srtp)
                        {
                         if(ssl_parse_serverhello_use_srtp_ext(s, data, size,
                                                              al))