Skip to content

Commit 2b0532f

Browse files
mattcaswellGeoff Thorpe
authored and
Geoff Thorpe
committed
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>
1 parent 7d07c75 commit 2b0532f

File tree

2 files changed

+36
-66
lines changed

2 files changed

+36
-66
lines changed

Diff for: ssl/d1_srtp.c

+31-62
Original file line numberDiff line numberDiff line change
@@ -168,25 +168,6 @@ static int find_profile_by_name(char *profile_name,
168168
return 1;
169169
}
170170

171-
static int find_profile_by_num(unsigned profile_num,
172-
SRTP_PROTECTION_PROFILE **pptr)
173-
{
174-
SRTP_PROTECTION_PROFILE *p;
175-
176-
p=srtp_known_profiles;
177-
while(p->name)
178-
{
179-
if(p->id == profile_num)
180-
{
181-
*pptr=p;
182-
return 0;
183-
}
184-
p++;
185-
}
186-
187-
return 1;
188-
}
189-
190171
static int ssl_ctx_make_profiles(const char *profiles_string,STACK_OF(SRTP_PROTECTION_PROFILE) **out)
191172
{
192173
STACK_OF(SRTP_PROTECTION_PROFILE) *profiles;
@@ -209,11 +190,19 @@ static int ssl_ctx_make_profiles(const char *profiles_string,STACK_OF(SRTP_PROTE
209190
if(!find_profile_by_name(ptr,&p,
210191
col ? col-ptr : (int)strlen(ptr)))
211192
{
193+
if (sk_SRTP_PROTECTION_PROFILE_find(profiles,p) >= 0)
194+
{
195+
SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
196+
sk_SRTP_PROTECTION_PROFILE_free(profiles);
197+
return 1;
198+
}
199+
212200
sk_SRTP_PROTECTION_PROFILE_push(profiles,p);
213201
}
214202
else
215203
{
216204
SSLerr(SSL_F_SSL_CTX_MAKE_PROFILES,SSL_R_SRTP_UNKNOWN_PROTECTION_PROFILE);
205+
sk_SRTP_PROTECTION_PROFILE_free(profiles);
217206
return 1;
218207
}
219208

@@ -305,13 +294,12 @@ int ssl_add_clienthello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int max
305294

306295
int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al)
307296
{
308-
SRTP_PROTECTION_PROFILE *cprof,*sprof;
309-
STACK_OF(SRTP_PROTECTION_PROFILE) *clnt=0,*srvr;
297+
SRTP_PROTECTION_PROFILE *sprof;
298+
STACK_OF(SRTP_PROTECTION_PROFILE) *srvr;
310299
int ct;
311300
int mki_len;
312-
int i,j;
313-
int id;
314-
int ret;
301+
int i, srtp_pref;
302+
unsigned int id;
315303

316304
/* Length value + the MKI length */
317305
if(len < 3)
@@ -341,22 +329,32 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
341329
return 1;
342330
}
343331

332+
srvr=SSL_get_srtp_profiles(s);
333+
s->srtp_profile = NULL;
334+
/* Search all profiles for a match initially */
335+
srtp_pref = sk_SRTP_PROTECTION_PROFILE_num(srvr);
344336

345-
clnt=sk_SRTP_PROTECTION_PROFILE_new_null();
346-
347337
while(ct)
348338
{
349339
n2s(d,id);
350340
ct-=2;
351341
len-=2;
352342

353-
if(!find_profile_by_num(id,&cprof))
343+
/*
344+
* Only look for match in profiles of higher preference than
345+
* current match.
346+
* If no profiles have been have been configured then this
347+
* does nothing.
348+
*/
349+
for (i = 0; i < srtp_pref; i++)
354350
{
355-
sk_SRTP_PROTECTION_PROFILE_push(clnt,cprof);
356-
}
357-
else
358-
{
359-
; /* Ignore */
351+
sprof = sk_SRTP_PROTECTION_PROFILE_value(srvr, i);
352+
if (sprof->id == id)
353+
{
354+
s->srtp_profile = sprof;
355+
srtp_pref = i;
356+
break;
357+
}
360358
}
361359
}
362360

@@ -371,36 +369,7 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al
371369
return 1;
372370
}
373371

374-
srvr=SSL_get_srtp_profiles(s);
375-
376-
/* Pick our most preferred profile. If no profiles have been
377-
configured then the outer loop doesn't run
378-
(sk_SRTP_PROTECTION_PROFILE_num() = -1)
379-
and so we just return without doing anything */
380-
for(i=0;i<sk_SRTP_PROTECTION_PROFILE_num(srvr);i++)
381-
{
382-
sprof=sk_SRTP_PROTECTION_PROFILE_value(srvr,i);
383-
384-
for(j=0;j<sk_SRTP_PROTECTION_PROFILE_num(clnt);j++)
385-
{
386-
cprof=sk_SRTP_PROTECTION_PROFILE_value(clnt,j);
387-
388-
if(cprof->id==sprof->id)
389-
{
390-
s->srtp_profile=sprof;
391-
*al=0;
392-
ret=0;
393-
goto done;
394-
}
395-
}
396-
}
397-
398-
ret=0;
399-
400-
done:
401-
if(clnt) sk_SRTP_PROTECTION_PROFILE_free(clnt);
402-
403-
return ret;
372+
return 0;
404373
}
405374

406375
int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int maxlen)

Diff for: ssl/t1_lib.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c
643643
#endif
644644

645645
#ifndef OPENSSL_NO_SRTP
646-
if(SSL_get_srtp_profiles(s))
646+
if(SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s))
647647
{
648648
int el;
649649

@@ -806,7 +806,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c
806806
#endif
807807

808808
#ifndef OPENSSL_NO_SRTP
809-
if(s->srtp_profile)
809+
if(SSL_IS_DTLS(s) && s->srtp_profile)
810810
{
811811
int el;
812812

@@ -1444,7 +1444,8 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in
14441444

14451445
/* session ticket processed earlier */
14461446
#ifndef OPENSSL_NO_SRTP
1447-
else if (type == TLSEXT_TYPE_use_srtp)
1447+
else if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)
1448+
&& type == TLSEXT_TYPE_use_srtp)
14481449
{
14491450
if(ssl_parse_clienthello_use_srtp_ext(s, data, size,
14501451
al))
@@ -1698,7 +1699,7 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in
16981699
}
16991700
#endif
17001701
#ifndef OPENSSL_NO_SRTP
1701-
else if (type == TLSEXT_TYPE_use_srtp)
1702+
else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_use_srtp)
17021703
{
17031704
if(ssl_parse_serverhello_use_srtp_ext(s, data, size,
17041705
al))

0 commit comments

Comments
 (0)