Skip to content

Commit

Permalink
A memory leak can occur in dtls1_buffer_record if either of the calls to
Browse files Browse the repository at this point in the history
ssl3_setup_buffers or pqueue_insert fail. The former will fail if there is a
malloc failure, whilst the latter will fail if attempting to add a duplicate
record to the queue. This should never happen because duplicate records should
be detected and dropped before any attempt to add them to the queue.
Unfortunately records that arrive that are for the next epoch are not being
recorded correctly, and therefore replays are not being detected.
Additionally, these "should not happen" failures that can occur in
dtls1_buffer_record are not being treated as fatal and therefore an attacker
could exploit this by sending repeated replay records for the next epoch,
eventually causing a DoS through memory exhaustion.

Thanks to Chris Mueller for reporting this issue and providing initial
analysis and a patch. Further analysis and the final patch was performed by
Matt Caswell from the OpenSSL development team.

CVE-2015-0206

Reviewed-by: Dr Stephen Henson <steve@openssl.org>
  • Loading branch information
mattcaswell committed Jan 8, 2015
1 parent 98a0f96 commit 04685bc
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions ssl/d1_pkt.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
/* Limit the size of the queue to prevent DOS attacks */
if (pqueue_size(queue->q) >= 100)
return 0;

rdata = OPENSSL_malloc(sizeof(DTLS1_RECORD_DATA));
item = pitem_new(priority, rdata);
if (rdata == NULL || item == NULL)
Expand Down Expand Up @@ -247,18 +247,22 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
if (!ssl3_setup_buffers(s))
{
SSLerr(SSL_F_DTLS1_BUFFER_RECORD, ERR_R_INTERNAL_ERROR);
if (rdata->rbuf.buf != NULL)
OPENSSL_free(rdata->rbuf.buf);
OPENSSL_free(rdata);
pitem_free(item);
return(0);
return(-1);
}

/* insert should not fail, since duplicates are dropped */
if (pqueue_insert(queue->q, item) == NULL)
{
SSLerr(SSL_F_DTLS1_BUFFER_RECORD, ERR_R_INTERNAL_ERROR);
if (rdata->rbuf.buf != NULL)
OPENSSL_free(rdata->rbuf.buf);
OPENSSL_free(rdata);
pitem_free(item);
return(0);
return(-1);
}

return(1);
Expand Down Expand Up @@ -314,8 +318,9 @@ dtls1_process_buffered_records(SSL *s)
dtls1_get_unprocessed_record(s);
if ( ! dtls1_process_record(s))
return(0);
dtls1_buffer_record(s, &(s->d1->processed_rcds),
s->s3->rrec.seq_num);
if(dtls1_buffer_record(s, &(s->d1->processed_rcds),
s->s3->rrec.seq_num)<0)
return -1;
}
}

Expand Down Expand Up @@ -530,7 +535,6 @@ printf("\n");

/* we have pulled in a full packet so zero things */
s->packet_length=0;
dtls1_record_bitmap_update(s, &(s->d1->bitmap));/* Mark receipt of record. */
return(1);

f_err:
Expand Down Expand Up @@ -563,7 +567,8 @@ int dtls1_get_record(SSL *s)

/* The epoch may have changed. If so, process all the
* pending records. This is a non-blocking operation. */
dtls1_process_buffered_records(s);
if(dtls1_process_buffered_records(s)<0)
return -1;

/* if we're renegotiating, then there may be buffered records */
if (dtls1_get_processed_record(s))
Expand Down Expand Up @@ -700,7 +705,9 @@ int dtls1_get_record(SSL *s)
{
if ((SSL_in_init(s) || s->in_handshake) && !s->d1->listen)
{
dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), rr->seq_num);
if(dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), rr->seq_num)<0)
return -1;
dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */
}
rr->length = 0;
s->packet_length = 0;
Expand All @@ -713,6 +720,7 @@ int dtls1_get_record(SSL *s)
s->packet_length = 0; /* dump this record */
goto again; /* get another record */
}
dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */

return(1);

Expand Down Expand Up @@ -864,7 +872,11 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
* buffer the application data for later processing rather
* than dropping the connection.
*/
dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num);
if(dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num)<0)
{
SSLerr(SSL_F_DTLS1_READ_BYTES, ERR_R_INTERNAL_ERROR);
return -1;
}
rr->length = 0;
goto start;
}
Expand Down

0 comments on commit 04685bc

Please sign in to comment.