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>

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index b8e6c73a244ba4e57ea722bc6748bfadcc6bb752..dddaa7e7c359f338546d3271e4c56050152f0a26 100644 (file)
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -611,6 +611,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 */
        memset(seq64be,0,sizeof(seq64be));
        seq64be[6] = (unsigned char) (msg_hdr->seq>>8);
@@ -688,7 +691,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;
@@ -746,7 +754,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))
@@ -775,7 +783,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;