Zero memory in CRYPTO_secure_malloc.
authorPauli <paul.dale@oracle.com>
Tue, 21 Aug 2018 23:20:18 +0000 (09:20 +1000)
committerPauli <paul.dale@oracle.com>
Tue, 21 Aug 2018 23:20:18 +0000 (09:20 +1000)
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@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/7011)

crypto/mem_sec.c
test/secmemtest.c

index 959c637..c4190be 100644 (file)
@@ -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;
 
index 9efa2c8..2795abb 100644 (file)
@@ -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;
 }