Skip Menu |
 
Ticket metadata
The Basics
Id: 1931
Status: resolved
Priority: 0/
Queue: OpenSSL-Bugs

Custom Fields
Milestone: (no value)
Subsystem: (no value)
Severity: (no value)
Broken in: (no value)

People
Owner: Stephen Henson
Requestors: Robin Seggelmann
Cc:
AdminCc:

Attachments
New reminder:
Subject:
Owner:
Due:

Dates
Created: Sat May 16 13:31:45 2009
Starts: Not set
Started: Sat May 16 18:28:06 2009
Last Contact: Sun May 17 18:46:05 2009
Due: Not set
Closed: Sun May 31 19:25:04 2009
Updated: Sun May 31 19:25:05 2009 by Stephen Henson



Subject: [PATCH] DTLS fragment handling memory leak
Date: Tue, 12 May 2009 17:38:00 +0200
To: rt@openssl.org
From: Robin Seggelmann <seggelmann@fh-muenster.de>
Download (untitled) / with headers
text/plain 1.3k
In dtls1_process_out_of_seq_message() the check if the current message
is already buffered was missing. For every new message was memory
allocated, allowing an attacker to perform an denial of service attack
with sending out of seq handshake messages until there is no memory
left. Additionally every future messege was buffered, even if the
sequence number made no sense and would be part of another handshake.
So only messages with sequence numbers less than 10 in advance will be
buffered.


Thanks to Daniel Mentz for finding this bug!



--- ssl/d1_both.c 2009-04-19 20:03:11.000000000 +0200
+++ ssl/d1_both.c 2009-05-12 10:15:35.000000000 +0200
@@ -561,7 +561,16 @@
if ((msg_hdr->frag_off+frag_len) > msg_hdr->msg_len)
goto err;

- if (msg_hdr->seq <= s->d1->handshake_read_seq)
+ /* Try to find item in queue, to prevent duplicate entries */
+ memset(seq64be,0,sizeof(seq64be));
+ seq64be[6] = (unsigned char) (msg_hdr->seq>>8);
+ seq64be[7] = (unsigned char) msg_hdr->seq;
+ item = pqueue_find(s->d1->buffered_messages, seq64be);
+
+ /* Discard the message if sequence number was already there, is
+ * too far in the future or the fragment is already in the queue */
+ if (msg_hdr->seq <= s->d1->handshake_read_seq ||
+ msg_hdr->seq > s->d1->handshake_read_seq + 10 || item != NULL)
{
unsigned char devnull [256];
Patch applied to 1.0, HEAD and 0.9.8. Thanks for the report.
Download (untitled) / with headers
text/plain 190b
Show quoted text
> [steve - Sat May 16 18:28:06 2009]:
>
> Patch applied to 1.0, HEAD and 0.9.8. Thanks for the report.
>

Patch reverted on 0.9.8, it breaks compilation. Please supply a version
for 0.9.8.
Subject: Re: [openssl.org #1931] [PATCH] DTLS fragment handling memory leak
Date: Mon, 18 May 2009 10:21:20 +0200
To: rt@openssl.org
From: Robin Seggelmann <seggelmann@fh-muenster.de>

On May 17, 2009, at 6:46 PM, Stephen Henson via RT wrote:

Show quoted text
>> [steve - Sat May 16 18:28:06 2009]:
>>
>> Patch applied to 1.0, HEAD and 0.9.8. Thanks for the report.
>>
>
> Patch reverted on 0.9.8, it breaks compilation. Please supply a
> version
> for 0.9.8.
>


The 0.9.8 version:


--- ssl/d1_both.c 2009-05-18 09:57:08.000000000 +0200
+++ ssl/d1_both.c 2009-05-18 10:08:51.000000000 +0200
@@ -561,7 +561,16 @@
if ((msg_hdr->frag_off+frag_len) > msg_hdr->msg_len)
goto err;

- if (msg_hdr->seq <= s->d1->handshake_read_seq)
+ /* Try to find item in queue, to prevent duplicate entries */
+ pq_64bit_init(&seq64);
+ pq_64bit_assign_word(&seq64, msg_hdr->seq);
+ item = pqueue_find(s->d1->buffered_messages, seq64);
+ pq_64bit_free(&seq64);
+
+ /* Discard the message if sequence number was already there, is
+ * too far in the future or the fragment is already in the queue */
+ if (msg_hdr->seq <= s->d1->handshake_read_seq ||
+ msg_hdr->seq > s->d1->handshake_read_seq + 10 || item != NULL)
{
unsigned char devnull [256];
Download dtls-fragment-memleak-bug.patch
application/octet-stream 758b

Message body not shown because it is not plain text.

Patches applied, ticket resolved.