summaryrefslogtreecommitdiffstats
path: root/ssl/d1_both.c
diff options
context:
space:
mode:
authorAdam Langley <agl@imperialviolet.org>2014-06-06 14:30:33 -0700
committerMatt Caswell <matt@openssl.org>2014-08-06 20:41:23 +0100
commit8ca4c4b25e050b881f3aad7017052842b888722d (patch)
tree7918c4c1481382b5f446b8564ea3a553b8f65a66 /ssl/d1_both.c
parent0598468fc04fb0cf2438c4ee635b587aac1bcce6 (diff)
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>
Diffstat (limited to 'ssl/d1_both.c')
-rw-r--r--ssl/d1_both.c22
1 files changed, 19 insertions, 3 deletions
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index c5beea8824..4d8d2d15ba 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -616,6 +616,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);
@@ -693,7 +696,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;
@@ -751,7 +759,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))
@@ -780,7 +788,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;