Skip to content

Commit 445598b

Browse files
aglmattcaswell
authored andcommitted
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>
1 parent 338a5e7 commit 445598b

File tree

1 file changed

+19
-3
lines changed

1 file changed

+19
-3
lines changed

ssl/d1_both.c

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,9 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
605605
msg_hdr->msg_len > dtls1_max_handshake_message_len(s))
606606
goto err;
607607

608+
if (frag_len == 0)
609+
return DTLS1_HM_FRAGMENT_RETRY;
610+
608611
/* Try to find item in queue */
609612
pq_64bit_init(&seq64);
610613
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)
682685
goto err;
683686
}
684687

685-
pqueue_insert(s->d1->buffered_messages, item);
688+
item = pqueue_insert(s->d1->buffered_messages, item);
689+
/* pqueue_insert fails iff a duplicate item is inserted.
690+
* However, |item| cannot be a duplicate. If it were,
691+
* |pqueue_find|, above, would have returned it and control
692+
* would never have reached this branch. */
693+
OPENSSL_assert(item != NULL);
686694
}
687695

688696
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)
740748
}
741749
else
742750
{
743-
if (frag_len && frag_len < msg_hdr->msg_len)
751+
if (frag_len < msg_hdr->msg_len)
744752
return dtls1_reassemble_fragment(s, msg_hdr, ok);
745753

746754
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)
769777
if ( item == NULL)
770778
goto err;
771779

772-
pqueue_insert(s->d1->buffered_messages, item);
780+
item = pqueue_insert(s->d1->buffered_messages, item);
781+
/* pqueue_insert fails iff a duplicate item is inserted.
782+
* However, |item| cannot be a duplicate. If it were,
783+
* |pqueue_find|, above, would have returned it. Then, either
784+
* |frag_len| != |msg_hdr->msg_len| in which case |item| is set
785+
* to NULL and it will have been processed with
786+
* |dtls1_reassemble_fragment|, above, or the record will have
787+
* been discarded. */
788+
OPENSSL_assert(item != NULL);
773789
}
774790

775791
return DTLS1_HM_FRAGMENT_RETRY;

0 commit comments

Comments
 (0)