From 3e60d2a4587f3b126e8254d78c0975b7f5d7dcd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Thu, 10 Dec 2015 10:50:51 +0100
Subject: [PATCH] Fix potential double free in cert writing code

In case an entry with the given OID already exists in the list passed to
mbedtls_asn1_store_named_data() and there is not enough memory to allocate
room for the new value, the existing entry will be freed but the preceding
entry in the list will sill hold a pointer to it. (And the following entries
in the list are no longer reachable.) This results in memory leak or a double
free.

The issue is we want to leave the list in a consistent state on allocation
failure. (We could add a warning that the list is left in inconsistent state
when the function returns NULL, but behaviour changes that require more care
from the user are undesirable, especially in a stable branch.)

The chosen solution is a bit inefficient in that there is a time where both
blocks are allocated, but at least it's safe and this should trump efficiency
here: this code is only used for generating certificates, which is unlikely to
be done on very constrained devices, or to be in the critical loop of
anything. Also, the sizes involved should be fairly small anyway.

fixes #367
---
 ChangeLog           |  7 +++++++
 library/asn1write.c | 23 +++++++++++------------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4065d041f..c6b79f650 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 2.1.4 released 2015-12-xx
+
+Security
+   * Fix potential double free when mbedtls_asn1_store_named_data() fails to
+     allocate memory. Only used for certificate generation, not triggerable
+     remotely in SSL/TLS. Found by Rafał Przywara. #367
+
 = mbed TLS 2.1.3 released 2015-11-04
 
 Security
diff --git a/library/asn1write.c b/library/asn1write.c
index 456660d8b..00ed73c11 100644
--- a/library/asn1write.c
+++ b/library/asn1write.c
@@ -339,19 +339,18 @@ mbedtls_asn1_named_data *mbedtls_asn1_store_named_data( mbedtls_asn1_named_data
     }
     else if( cur->val.len < val_len )
     {
-        // Enlarge existing value buffer if needed
-        //
-        mbedtls_free( cur->val.p );
-        cur->val.p = NULL;
-
-        cur->val.len = val_len;
-        cur->val.p = mbedtls_calloc( 1, val_len );
-        if( cur->val.p == NULL )
-        {
-            mbedtls_free( cur->oid.p );
-            mbedtls_free( cur );
+        /*
+         * Enlarge existing value buffer if needed
+         * Preserve old data until the allocation succeeded, to leave list in
+         * a consistent state in case allocation fails.
+         */
+        void *p = mbedtls_calloc( 1, val_len );
+        if( p == NULL )
             return( NULL );
-        }
+
+        mbedtls_free( cur->val.p );
+        cur->val.p = p;
+        cur->val.len = val_len;
     }
 
     if( val != NULL )