Skip to content

Commit d0a4b7d

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 1250f12 commit d0a4b7d

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
@@ -616,6 +616,9 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
616616
msg_hdr->msg_len > dtls1_max_handshake_message_len(s))
617617
goto err;
618618

619+
if (frag_len == 0)
620+
return DTLS1_HM_FRAGMENT_RETRY;
621+
619622
/* Try to find item in queue */
620623
memset(seq64be,0,sizeof(seq64be));
621624
seq64be[6] = (unsigned char) (msg_hdr->seq>>8);
@@ -693,7 +696,12 @@ dtls1_reassemble_fragment(SSL *s, struct hm_header_st* msg_hdr, int *ok)
693696
goto err;
694697
}
695698

696-
pqueue_insert(s->d1->buffered_messages, item);
699+
item = pqueue_insert(s->d1->buffered_messages, item);
700+
/* pqueue_insert fails iff a duplicate item is inserted.
701+
* However, |item| cannot be a duplicate. If it were,
702+
* |pqueue_find|, above, would have returned it and control
703+
* would never have reached this branch. */
704+
OPENSSL_assert(item != NULL);
697705
}
698706

699707
return DTLS1_HM_FRAGMENT_RETRY;
@@ -751,7 +759,7 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok)
751759
}
752760
else
753761
{
754-
if (frag_len && frag_len < msg_hdr->msg_len)
762+
if (frag_len < msg_hdr->msg_len)
755763
return dtls1_reassemble_fragment(s, msg_hdr, ok);
756764

757765
if (frag_len > dtls1_max_handshake_message_len(s))
@@ -780,7 +788,15 @@ dtls1_process_out_of_seq_message(SSL *s, struct hm_header_st* msg_hdr, int *ok)
780788
if ( item == NULL)
781789
goto err;
782790

783-
pqueue_insert(s->d1->buffered_messages, item);
791+
item = pqueue_insert(s->d1->buffered_messages, item);
792+
/* pqueue_insert fails iff a duplicate item is inserted.
793+
* However, |item| cannot be a duplicate. If it were,
794+
* |pqueue_find|, above, would have returned it. Then, either
795+
* |frag_len| != |msg_hdr->msg_len| in which case |item| is set
796+
* to NULL and it will have been processed with
797+
* |dtls1_reassemble_fragment|, above, or the record will have
798+
* been discarded. */
799+
OPENSSL_assert(item != NULL);
784800
}
785801

786802
return DTLS1_HM_FRAGMENT_RETRY;

0 commit comments

Comments
 (0)