[openssl-commits] [openssl] master update

[Paul I. Dale]

The branch master has been updated
       via  3b8e97ab61624f4fbe8bb6a587f4da75cc3d988e (commit)
      from  d41a8319272968596a5daa1870007f2adf1e75ee (commit)


- Log -----------------------------------------------------------------
commit 3b8e97ab61624f4fbe8bb6a587f4da75cc3d988e
Author: Pauli <paul.dale at oracle.com>
Date:   Wed Aug 22 09:20:18 2018 +1000

    Zero memory in CRYPTO_secure_malloc.
    
    This commit destroys the free list pointers which would otherwise be
    present in the returned memory blocks.  This in turn helps prevent
    information leakage from the secure memory area.
    
    Note: CRYPTO_secure_malloc is not guaranteed to return zeroed memory:
    before the secure memory system is initialised or if it isn't implemented.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
    (Merged from https://github.com/openssl/openssl/pull/7011)

-----------------------------------------------------------------------

Summary of changes:
 crypto/mem_sec.c  | 16 +++++++++++-----
 test/secmemtest.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c
index 959c637..c4190be 100644
--- a/crypto/mem_sec.c
+++ b/crypto/mem_sec.c
@@ -137,11 +137,12 @@ void *CRYPTO_secure_malloc(size_t num, const char *file, int line)
 
 void *CRYPTO_secure_zalloc(size_t num, const char *file, int line)
 {
-    void *ret = CRYPTO_secure_malloc(num, file, line);
-
-    if (ret != NULL)
-        memset(ret, 0, num);
-    return ret;
+#ifdef IMPLEMENTED
+    if (secure_mem_initialized)
+        /* CRYPTO_secure_malloc() zeroes allocations when it is implemented */
+        return CRYPTO_secure_malloc(num, file, line);
+#endif
+    return CRYPTO_zalloc(num, file, line);
 }
 
 void CRYPTO_secure_free(void *ptr, const char *file, int line)
@@ -588,6 +589,9 @@ static void *sh_malloc(size_t size)
 
     OPENSSL_assert(WITHIN_ARENA(chunk));
 
+    /* zero the free list header as a precaution against information leakage */
+    memset(chunk, 0, sizeof(SH_LIST));
+
     return chunk;
 }
 
@@ -620,6 +624,8 @@ static void sh_free(void *ptr)
 
         list--;
 
+        /* Zero the higher addressed block's free list pointers */
+        memset(ptr > buddy ? ptr : buddy, 0, sizeof(SH_LIST));
         if (ptr > buddy)
             ptr = buddy;
 
diff --git a/test/secmemtest.c b/test/secmemtest.c
index 9efa2c8..2795abb 100644
--- a/test/secmemtest.c
+++ b/test/secmemtest.c
@@ -129,8 +129,52 @@ static int test_sec_mem(void)
 #endif
 }
 
+static int test_sec_mem_clear(void)
+{
+#if defined(OPENSSL_SYS_LINUX) || defined(OPENSSL_SYS_UNIX)
+    const int size = 64;
+    unsigned char *p = NULL;
+    int i, res = 0;
+
+    if (!TEST_true(CRYPTO_secure_malloc_init(4096, 32))
+            || !TEST_ptr(p = OPENSSL_secure_malloc(size)))
+        goto err;
+
+    for (i = 0; i < size; i++)
+        if (!TEST_uchar_eq(p[i], 0))
+            goto err;
+
+    for (i = 0; i < size; i++)
+        p[i] = (unsigned char)(i + ' ' + 1);
+
+    OPENSSL_secure_free(p);
+
+    /*
+     * A deliberate use after free here to verify that the memory has been
+     * cleared properly.  Since secure free doesn't return the memory to
+     * libc's memory pool, it technically isn't freed.  However, the header
+     * bytes have to be skipped and these consist of two pointers in the
+     * current implementation.
+     */
+    for (i = sizeof(void *) * 2; i < size; i++)
+        if (!TEST_uchar_eq(p[i], 0))
+            return 0;
+
+    res = 1;
+    p = NULL;
+
+err:
+    OPENSSL_secure_free(p);
+    CRYPTO_secure_malloc_done();
+    return res;
+#else
+    return 1;
+#endif
+}
+
 int setup_tests(void)
 {
     ADD_TEST(test_sec_mem);
+    ADD_TEST(test_sec_mem_clear);
     return 1;
 }