Skip to content

Commit 9871417

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 fc7804e commit 9871417

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
@@ -611,6 +611,9 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
611611
msg_hdr->msg_len > dtls1_max_handshake_message_len(s))
612612
goto err;
613613

614+
if (frag_len == 0)
615+
return DTLS1_HM_FRAGMENT_RETRY;
616+
614617
/* Try to find item in queue */
615618
memset(seq64be,0,sizeof(seq64be));
616619
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)
688691
goto err;
689692
}
690693

691-
pqueue_insert(s->d1->buffered_messages, item);
694+
item = pqueue_insert(s->d1->buffered_messages, item);
695+
/* pqueue_insert fails iff a duplicate item is inserted.
696+
* However, |item| cannot be a duplicate. If it were,
697+
* |pqueue_find|, above, would have returned it and control
698+
* would never have reached this branch. */
699+
OPENSSL_assert(item != NULL);
692700
}
693701

694702
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)
746754
}
747755
else
748756
{
749-
if (frag_len && frag_len < msg_hdr->msg_len)
757+
if (frag_len < msg_hdr->msg_len)
750758
return dtls1_reassemble_fragment(s, msg_hdr, ok);
751759

752760
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)
775783
if ( item == NULL)
776784
goto err;
777785

778-
pqueue_insert(s->d1->buffered_messages, item);
786+
item = pqueue_insert(s->d1->buffered_messages, item);
787+
/* pqueue_insert fails iff a duplicate item is inserted.
788+
* However, |item| cannot be a duplicate. If it were,
789+
* |pqueue_find|, above, would have returned it. Then, either
790+
* |frag_len| != |msg_hdr->msg_len| in which case |item| is set
791+
* to NULL and it will have been processed with
792+
* |dtls1_reassemble_fragment|, above, or the record will have
793+
* been discarded. */
794+
OPENSSL_assert(item != NULL);
779795
}
780796

781797
return DTLS1_HM_FRAGMENT_RETRY;

0 commit comments

Comments
 (0)