From 445598b35e16090b676bb168807da06518658b34 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Fri, 6 Jun 2014 14:30:33 -0700 Subject: [PATCH] Fix memory leak from zero-length DTLS fragments. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The |pqueue_insert| function can fail if one attempts to insert a duplicate sequence number. When handling a fragment of an out of sequence message, |dtls1_process_out_of_seq_message| would not call |dtls1_reassemble_fragment| if the fragment's length was zero. It would then allocate a fresh fragment and attempt to insert it, but ignore the return value, leaking the fragment. This allows an attacker to exhaust the memory of a DTLS peer. Fixes CVE-2014-3507 Reviewed-by: Matt Caswell Reviewed-by: Emilia Käsper --- ssl/d1_both.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/ssl/d1_both.c b/ssl/d1_both.c index e0eed129b8..99325e8f04 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -605,6 +605,9 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok) msg_hdr->msg_len > dtls1_max_handshake_message_len(s)) goto err; + if (frag_len == 0) + return DTLS1_HM_FRAGMENT_RETRY; + /* Try to find item in queue */ pq_64bit_init(&seq64); pq_64bit_assign_word(&seq64, msg_hdr->seq); @@ -682,7 +685,12 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok) goto err; } - pqueue_insert(s->d1->buffered_messages, item); + item = pqueue_insert(s->d1->buffered_messages, item); + /* pqueue_insert fails iff a duplicate item is inserted. + * However, |item| cannot be a duplicate. If it were, + * |pqueue_find|, above, would have returned it and control + * would never have reached this branch. */ + OPENSSL_assert(item != NULL); } return DTLS1_HM_FRAGMENT_RETRY; @@ -740,7 +748,7 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok) } else { - if (frag_len && frag_len < msg_hdr->msg_len) + if (frag_len < msg_hdr->msg_len) return dtls1_reassemble_fragment(s, msg_hdr, ok); if (frag_len > dtls1_max_handshake_message_len(s)) @@ -769,7 +777,15 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok) if ( item == NULL) goto err; - pqueue_insert(s->d1->buffered_messages, item); + item = pqueue_insert(s->d1->buffered_messages, item); + /* pqueue_insert fails iff a duplicate item is inserted. + * However, |item| cannot be a duplicate. If it were, + * |pqueue_find|, above, would have returned it. Then, either + * |frag_len| != |msg_hdr->msg_len| in which case |item| is set + * to NULL and it will have been processed with + * |dtls1_reassemble_fragment|, above, or the record will have + * been discarded. */ + OPENSSL_assert(item != NULL); } return DTLS1_HM_FRAGMENT_RETRY; -- 2.25.1