Fix DTLS memory leak (CVE-2015-0206).
authordoug <doug@openbsd.org>
Wed, 21 Jan 2015 00:15:50 +0000 (00:15 +0000)
committerdoug <doug@openbsd.org>
Wed, 21 Jan 2015 00:15:50 +0000 (00:15 +0000)
commit3221bfb84da82a2f41c0eac30e55ef00cebd0cf1
tree19a015ac7605eb14591c5185918612135ec9a690
parentace78debd18ce723905dd08f8e9e5976c31613bc
Fix DTLS memory leak (CVE-2015-0206).

There were four bugs fixed by this patch:

* dtls1_buffer_record() now frees rdata->rbuf.buf on error.  Since
  s->s3->rbuf was memset, rdata->rbuf is the only pointer left which
  points to the old rbuf.  On error, rdata is freed so there will not
  be any way of freeing this memory unless we do it here.

* Changed the return code of dtls1_buffer_record() to differentiate
  between queue full (0) and error (-1).  See below as this differs
  from upstream.

* Handle errors if calls to dtls1_buffer_record() fail with -1.
  Previously, it did not check the return value.

* Changed the way receipts are recorded.  Previously, it was recorded
  when processed successfully (whether buffered or not) in
  dtls1_process_record().  Now, it records when it is handled in
  dtls1_get_record(): either when it is entered into the queue to buffer
  for the next epoch or when it is processed directly.  Processing
  buffered records does not add a receipt because it needed one in
  order to get into the queue.

The above bugs combined contributed to an eventual DoS through memory
exhaustion.  The memory leak came from dtls1_buffer_record()'s error
handling.  The error handling can be triggered by a duplicate record
or malloc failure.  It was possible to add duplicate records because
they were not being dropped.  The faulty receipts logic did not detect
replays when dealing with records for the next epoch.  Additionally,
dtls1_buffer_record()'s return value was not checked so an attacker
could send repeated replay records for the next epoch.

Reported to OpenSSL by Chris Mueller.

Patch based on OpenSSL commit 103b171d8fc282ef435f8de9afbf7782e312961f
and BoringSSL commit 44e2709cd65fbd2172b9516c79e56f1875f60300.

Our patch matches BoringSSL's commit.  OpenSSL returns 0 when the queue
is full or when malloc() or pitem_new() fails.  They return -1 on error
including !ssl3_setup_buffers() which is another failure to allocate
memory.

BoringSSL and LibreSSL changed the return code for dtls1_buffer_record()
to be 1 on success, 0 when the queue is full and -1 on error.

input + ok bcook@, jsing@
lib/libssl/d1_pkt.c
lib/libssl/src/ssl/d1_pkt.c