Merge remote-tracking branch 'upstream-public/pr/778' into development-proposed

7 files changed, 65 insertions, 15 deletions

diff --git a/ChangeLog b/ChangeLog
index dfd34bf6..ed2165e8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -44,6 +44,8 @@ Bugfix
      Nick Wilson on issue #355
    * In test_suite_pk, pass valid parameters when testing for hash length
      overflow. #1179
+   * Fix memory allocation corner cases in memory_buffer_alloc.c module. Found
+     by Guido Vranken. #639
 
 Changes
    * Fix tag lengths and value ranges in the documentation of CCM encryption.
diff --git a/library/memory_buffer_alloc.c b/library/memory_buffer_alloc.c
index 545d5a2c..1cfc27ca 100644
--- a/library/memory_buffer_alloc.c
+++ b/library/memory_buffer_alloc.c
@@ -182,9 +182,9 @@ static int verify_header( memory_header *hdr )
 
 static int verify_chain()
 {
-    memory_header *prv = heap.first, *cur = heap.first->next;
+    memory_header *prv = heap.first, *cur;
 
-    if( verify_header( heap.first ) != 0 )
+    if( prv == NULL || verify_header( prv ) != 0 )
     {
 #if defined(MBEDTLS_MEMORY_DEBUG)
         mbedtls_fprintf( stderr, "FATAL: verification of first header "
@@ -202,6 +202,8 @@ static int verify_chain()
         return( 1 );
     }
 
+    cur = heap.first->next;
+
     while( cur != NULL )
     {
         if( verify_header( cur ) != 0 )
@@ -245,7 +247,9 @@ static void *buffer_alloc_calloc( size_t n, size_t size )
 
     original_len = len = n * size;
 
-    if( n != 0 && len / n != size )
+    if( n == 0 || size == 0 || len / n != size )
+        return( NULL );
+    else if( len > (size_t)-MBEDTLS_MEMORY_ALIGN_MULTIPLE )
         return( NULL );
 
     if( len % MBEDTLS_MEMORY_ALIGN_MULTIPLE )
@@ -386,7 +390,7 @@ static void buffer_alloc_free( void *ptr )
     if( ptr == NULL || heap.buf == NULL || heap.first == NULL )
         return;
 
-    if( p < heap.buf || p > heap.buf + heap.len )
+    if( p < heap.buf || p >= heap.buf + heap.len )
     {
 #if defined(MBEDTLS_MEMORY_DEBUG)
         mbedtls_fprintf( stderr, "FATAL: mbedtls_free() outside of managed "
@@ -570,8 +574,7 @@ static void buffer_alloc_free_mutexed( void *ptr )
 
 void mbedtls_memory_buffer_alloc_init( unsigned char *buf, size_t len )
 {
-    memset( &heap, 0, sizeof(buffer_alloc_ctx) );
-    memset( buf, 0, len );
+    memset( &heap, 0, sizeof( buffer_alloc_ctx ) );
 
 #if defined(MBEDTLS_THREADING_C)
     mbedtls_mutex_init( &heap.mutex );
@@ -581,20 +584,24 @@ void mbedtls_memory_buffer_alloc_init( unsigned char *buf, size_t len )
     mbedtls_platform_set_calloc_free( buffer_alloc_calloc, buffer_alloc_free );
 #endif
 
-    if( (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE )
+    if( len < sizeof( memory_header ) + MBEDTLS_MEMORY_ALIGN_MULTIPLE )
+        return;
+    else if( (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE )
     {
         /* Adjust len first since buf is used in the computation */
         len -= MBEDTLS_MEMORY_ALIGN_MULTIPLE
-             - (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE;
+             - (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE;
         buf += MBEDTLS_MEMORY_ALIGN_MULTIPLE
-             - (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE;
+             - (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE;
     }
 
+    memset( buf, 0, len );
+
     heap.buf = buf;
     heap.len = len;
 
-    heap.first = (memory_header *) buf;
-    heap.first->size = len - sizeof(memory_header);
+    heap.first = (memory_header *)buf;
+    heap.first->size = len - sizeof( memory_header );
     heap.first->magic1 = MAGIC1;
     heap.first->magic2 = MAGIC2;
     heap.first_free = heap.first;
diff --git a/library/pem.c b/library/pem.c
index c09651f4..ac86d7e4 100644
--- a/library/pem.c
+++ b/library/pem.c
@@ -442,7 +442,7 @@ int mbedtls_pem_write_buffer( const char *header, const char *footer,
                       unsigned char *buf, size_t buf_len, size_t *olen )
 {
     int ret;
-    unsigned char *encode_buf, *c, *p = buf;
+    unsigned char *encode_buf = NULL, *c, *p = buf;
     size_t len = 0, use_len, add_len = 0;
 
     mbedtls_base64_encode( NULL, 0, &use_len, der_data, der_len );
@@ -454,7 +454,8 @@ int mbedtls_pem_write_buffer( const char *header, const char *footer,
         return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL );
     }
 
-    if( ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL )
+    if( use_len != 0 &&
+        ( ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL ) )
         return( MBEDTLS_ERR_PEM_ALLOC_FAILED );
 
     if( ( ret = mbedtls_base64_encode( encode_buf, use_len, &use_len, der_data,
diff --git a/library/pkparse.c b/library/pkparse.c
index aae17854..a0f77794 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -1277,6 +1277,9 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk,
     {
         unsigned char *key_copy;
 
+        if( keylen == 0 )
+            return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT );
+
         if( ( key_copy = mbedtls_calloc( 1, keylen ) ) == NULL )
             return( MBEDTLS_ERR_PK_ALLOC_FAILED );
 
diff --git a/library/x509_crl.c b/library/x509_crl.c
index 55d12acd..0bb7236b 100644
--- a/library/x509_crl.c
+++ b/library/x509_crl.c
@@ -257,7 +257,7 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain,
 {
     int ret;
     size_t len;
-    unsigned char *p, *end;
+    unsigned char *p = NULL, *end = NULL;
     mbedtls_x509_buf sig_params1, sig_params2, sig_oid2;
     mbedtls_x509_crl *crl = chain;
 
@@ -294,7 +294,11 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain,
     /*
      * Copy raw DER-encoded CRL
      */
-    if( ( p = mbedtls_calloc( 1, buflen ) ) == NULL )
+    if( buflen == 0 )
+        return( MBEDTLS_ERR_X509_INVALID_FORMAT );
+
+    p = mbedtls_calloc( 1, buflen );
+    if( p == NULL )
         return( MBEDTLS_ERR_X509_ALLOC_FAILED );
 
     memcpy( p, buf, buflen );
diff --git a/tests/suites/test_suite_memory_buffer_alloc.data b/tests/suites/test_suite_memory_buffer_alloc.data
index 8d3813a7..d59f1135 100644
--- a/tests/suites/test_suite_memory_buffer_alloc.data
+++ b/tests/suites/test_suite_memory_buffer_alloc.data
@@ -16,3 +16,8 @@ memory_buffer_alloc_free_alloc:100:64:100:100:0:0:0:1:200:0
 Memory buffer alloc - Out of Memory test
 memory_buffer_alloc_oom_test:
 
+Memory buffer small buffer
+memory_buffer_small_buffer:
+
+Memory buffer underalloc
+memory_buffer_underalloc:
diff --git a/tests/suites/test_suite_memory_buffer_alloc.function b/tests/suites/test_suite_memory_buffer_alloc.function
index a0c70d8a..09684c1d 100644
--- a/tests/suites/test_suite_memory_buffer_alloc.function
+++ b/tests/suites/test_suite_memory_buffer_alloc.function
@@ -232,3 +232,31 @@ exit:
 }
 /* END_CASE */
 
+/* BEGIN_CASE depends_on:MBEDTLS_MEMORY_DEBUG */
+void memory_buffer_small_buffer( )
+{
+    unsigned char buf[1];
+
+    mbedtls_memory_buffer_alloc_init( buf, sizeof( buf ) );
+    TEST_ASSERT( mbedtls_memory_buffer_alloc_verify() != 0 );
+}
+/* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_MEMORY_DEBUG */
+void memory_buffer_underalloc( )
+{
+    unsigned char buf[100];
+    size_t i;
+
+    mbedtls_memory_buffer_alloc_init( buf, sizeof( buf ) );
+    for( i = 1; i < MBEDTLS_MEMORY_ALIGN_MULTIPLE; i++ )
+    {
+        TEST_ASSERT( mbedtls_calloc( 1,
+                     (size_t)-( MBEDTLS_MEMORY_ALIGN_MULTIPLE - i ) ) == NULL );
+        TEST_ASSERT( mbedtls_memory_buffer_alloc_verify() == 0 );
+    }
+
+exit:
+    mbedtls_memory_buffer_alloc_free();
+}
+/* END_CASE */