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.

[manual merge of #7011]

Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/7026)

diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c
index 25cdb47d56c0de6a648d9acfde168e6e56900eba..1ccf68cc9335b06420273be683026f18e39c9b98 100644 (file)
--- a/crypto/mem_sec.c
+++ b/crypto/mem_sec.c
@@ -134,11 +134,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)
@@ -574,6 +575,9 @@ static char *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;
 }
 
@@ -606,6 +610,8 @@ static void sh_free(char *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 9405f348abc5c11fb6aa314851248f43e57b47ca..6077216b9b4aeed49f5be0c662bb7bbd7d9a0d09 100644 (file)
--- a/test/secmemtest.c
+++ b/test/secmemtest.c
@@ -18,6 +18,8 @@ int main(int argc, char **argv)
 {
 #if defined(OPENSSL_SYS_LINUX) || defined(OPENSSL_SYS_UNIX)
     char *p = NULL, *q = NULL, *r = NULL, *s = NULL;
+    int i;
+    const int size = 64;
 
     s = OPENSSL_secure_malloc(20);
     /* s = non-secure 20 */
@@ -128,6 +130,48 @@ int main(int argc, char **argv)
         return 1;
     }
 
+    if (!CRYPTO_secure_malloc_init(32768, 16)) {
+        perror_line();
+        return 1;
+    }
+
+    /*
+     * Verify that secure memory gets zeroed properly.
+     */
+    if ((p = OPENSSL_secure_malloc(size)) == NULL) {
+        perror_line();
+        return 1;
+    }
+    for (i = 0; i < size; i++)
+        if (p[i] != 0) {
+            perror_line();
+            fprintf(stderr, "iteration %d\n", i);
+            return 1;
+        }
+
+    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 (p[i] != 0) {
+            perror_line();
+            fprintf(stderr, "iteration %d\n", i);
+            return 1;
+        }
+
+    if (!CRYPTO_secure_malloc_done()) {
+        perror_line();
+        return 1;
+    }
+
     /*-
      * There was also a possible infinite loop when the number of
      * elements was 1<<31, as |int i| was set to that, which is a