CVE-2015-0206: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Memory leak in the dtls1_buffer_record function in d1_pkt.c in OpenSSL 1.0.0 before 1.0.0p and 1.0.1 before 1.0.1k allows remote attackers to cause a denial of service (memory consumption) by sending many duplicate records for the next epoch, leading to failure of replay detection.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2015-01-09T02:59Z
Repository: https://github.com/openssl/openssl
Language: C
Files changed: ssl/d1_pkt.c
Commit date: 2015-01-08 15:49:45 +0000
Commit message: 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>

Diff: diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 1028e41b56..a8a06b439b 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -210,11 +210,11 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
 	pitem *item;
 
 	/* 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)
 		{
 		if (rdata != NULL) OPENSSL_free(rdata);
@@ -245,22 +245,26 @@ dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority)
 	memset(&(s->s3->rrec), 0, sizeof(SSL3_RECORD));
 	
 	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);
 	}
 
@@ -312,12 +316,13 @@ dtls1_process_buffered_records(SSL *s)
         while (pqueue_peek(s->d1->unprocessed_rcds.q))
             {
             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;
             }
         }
 
     /* sync epoch numbers once all the unprocessed records 
      * have been processed */
@@ -527,11 +532,10 @@ printf("\n");
 	 *			   after use :-).
 	 */
 
 	/* 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:
 	ssl3_send_alert(s,SSL3_AL_FATAL,al);
 err:
@@ -561,11 +565,12 @@ int dtls1_get_record(SSL *s)
 
 	rr= &(s->s3->rrec);
 
 	/* 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))
 		return 1;
 
@@ -701,11 +706,13 @@ again:
 	 */
 	if (is_next_epoch)
 		{
 		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;
 		goto again;
 		}
@@ -714,10 +721,11 @@ again:
 		{
 		rr->length = 0;
 		s->packet_length = 0;  /* dump this record */
 		goto again;   /* get another record */
 		}
+	dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */
 
 	return(1);
 
 	}
 
@@ -867,11 +875,15 @@ start:
 		/* We now have application data between CCS and Finished.
 		 * Most likely the packets were reordered on their way, so
 		 * 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;
 		}
 
 	/* If the other end has shut down, throw anything we read away

Vulnerable code: 		
		return(0);
		return(0);
            dtls1_buffer_record(s, &(s->d1->processed_rcds), 
                s->s3->rrec.seq_num);
	dtls1_record_bitmap_update(s, &(s->d1->bitmap));/* Mark receipt of record. */
	dtls1_process_buffered_records(s);
			dtls1_buffer_record(s, &(s->d1->unprocessed_rcds), rr->seq_num);
		dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num);
Fixed code: 
		if (rdata->rbuf.buf != NULL)
			OPENSSL_free(rdata->rbuf.buf);
		return(-1);
		if (rdata->rbuf.buf != NULL)
			OPENSSL_free(rdata->rbuf.buf);
		return(-1);
            if(dtls1_buffer_record(s, &(s->d1->processed_rcds),
                s->s3->rrec.seq_num)<0)
                return -1;
	if(dtls1_process_buffered_records(s)<0)
		return -1;
			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. */
	dtls1_record_bitmap_update(s, bitmap);/* Mark receipt of record. */
		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;
			}