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>

(Imported from upstream's 8ca4c4b25e050b881f3aad7017052842b888722d.)

Change-Id: I387e3f6467a0041f6367965ed3c1ad4377b9ac08
Reviewed-on: https://boringssl-review.googlesource.com/1435
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
Adam Langley 2014-06-06 14:30:33 -07:00 committed by Adam Langley
parent 2306fe5ff5
commit e951ff4fc3

View File

@ -619,6 +619,9 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
msg_hdr->msg_len > dtls1_max_handshake_message_len(s)) msg_hdr->msg_len > dtls1_max_handshake_message_len(s))
goto err; goto err;
if (frag_len == 0)
return DTLS1_HM_FRAGMENT_RETRY;
/* Try to find item in queue */ /* Try to find item in queue */
memset(seq64be,0,sizeof(seq64be)); memset(seq64be,0,sizeof(seq64be));
seq64be[6] = (unsigned char) (msg_hdr->seq>>8); seq64be[6] = (unsigned char) (msg_hdr->seq>>8);
@ -695,7 +698,12 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
goto err; 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. */
assert(item != NULL);
} }
return DTLS1_HM_FRAGMENT_RETRY; return DTLS1_HM_FRAGMENT_RETRY;
@ -753,7 +761,7 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok)
} }
else 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); return dtls1_reassemble_fragment(s, msg_hdr, ok);
if (frag_len > dtls1_max_handshake_message_len(s)) if (frag_len > dtls1_max_handshake_message_len(s))
@ -782,7 +790,15 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok)
if ( item == NULL) if ( item == NULL)
goto err; 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. */
assert(item != NULL);
} }
return DTLS1_HM_FRAGMENT_RETRY; return DTLS1_HM_FRAGMENT_RETRY;