Skip to content

Commit 103b171

Browse files
committed
A memory leak can occur in dtls1_buffer_record if either of the calls to
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>
1 parent 1421e0c commit 103b171

File tree

1 file changed

+21
-9
lines changed

1 file changed

+21
-9
lines changed

Diff for: ssl/d1_pkt.c

+21-9
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
212212
/* Limit the size of the queue to prevent DOS attacks */
213213
if (pqueue_size(queue->q) >= 100)
214214
return 0;
215-
215+
216216
rdata = OPENSSL_malloc(sizeof(DTLS1_RECORD_DATA));
217217
item = pitem_new(priority, rdata);
218218
if (rdata == NULL || item == NULL)
@@ -247,18 +247,22 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
247247
if (!ssl3_setup_buffers(s))
248248
{
249249
SSLerr(SSL_F_DTLS1_BUFFER_RECORD, ERR_R_INTERNAL_ERROR);
250+
if (rdata->rbuf.buf != NULL)
251+
OPENSSL_free(rdata->rbuf.buf);
250252
OPENSSL_free(rdata);
251253
pitem_free(item);
252-
return(0);
254+
return(-1);
253255
}
254256

255257
/* insert should not fail, since duplicates are dropped */
256258
if (pqueue_insert(queue->q, item) == NULL)
257259
{
258260
SSLerr(SSL_F_DTLS1_BUFFER_RECORD, ERR_R_INTERNAL_ERROR);
261+
if (rdata->rbuf.buf != NULL)
262+
OPENSSL_free(rdata->rbuf.buf);
259263
OPENSSL_free(rdata);
260264
pitem_free(item);
261-
return(0);
265+
return(-1);
262266
}
263267

264268
return(1);
@@ -314,8 +318,9 @@ dtls1_process_buffered_records(SSL *s)
314318
dtls1_get_unprocessed_record(s);
315319
if ( ! dtls1_process_record(s))
316320
return(0);
317-
dtls1_buffer_record(s, &(s->d1->processed_rcds),
318-
s->s3->rrec.seq_num);
321+
if(dtls1_buffer_record(s, &(s->d1->processed_rcds),
322+
s->s3->rrec.seq_num)<0)
323+
return -1;
319324
}
320325
}
321326

@@ -529,7 +534,6 @@ printf("\n");
529534

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

535539
f_err:
@@ -563,7 +567,8 @@ int dtls1_get_record(SSL *s)
563567

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

568573
/* if we're renegotiating, then there may be buffered records */
569574
if (dtls1_get_processed_record(s))
@@ -703,7 +708,9 @@ int dtls1_get_record(SSL *s)
703708
{
704709
if ((SSL_in_init(s) || s->in_handshake) && !s->d1->listen)
705710
{
706-
dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), rr->seq_num);
711+
if(dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), rr->seq_num)<0)
712+
return -1;
713+
dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */
707714
}
708715
rr->length = 0;
709716
s->packet_length = 0;
@@ -716,6 +723,7 @@ int dtls1_get_record(SSL *s)
716723
s->packet_length = 0; /* dump this record */
717724
goto again; /* get another record */
718725
}
726+
dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */
719727

720728
return(1);
721729

@@ -869,7 +877,11 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
869877
* buffer the application data for later processing rather
870878
* than dropping the connection.
871879
*/
872-
dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num);
880+
if(dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num)<0)
881+
{
882+
SSLerr(SSL_F_DTLS1_READ_BYTES, ERR_R_INTERNAL_ERROR);
883+
return -1;
884+
}
873885
rr->length = 0;
874886
goto start;
875887
}

0 commit comments

Comments
 (0)