diff options
| author | Adam Langley <agl@imperialviolet.org> | 2014-06-06 14:30:33 -0700 |
|---|---|---|
| committer | Matt Caswell <matt@openssl.org> | 2014-08-06 22:02:00 +0100 |
| commit | 445598b35e16090b676bb168807da06518658b34 (patch) | |
| tree | 46d19f77b39de651fea35061964be3f7c3b877e7 /ssl | |
| parent | 338a5e7e5458edf4cf754fd831a451fb4b57d180 (diff) | |
Fix memory leak from zero-length DTLS fragments.
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 <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Diffstat (limited to 'ssl')
| -rw-r--r-- | ssl/d1_both.c | 22 |
1 files 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; |