Commit 837017b4 authored by Pauli's avatar Pauli
Browse files

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: default avatarMatthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/7026)
parent 8255fd0f

crypto/mem_sec.c

+11 −5
Original line number Diff line number Diff line
@@ -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;

test/secmemtest.c

+44 −0
Original line number Diff line number Diff line
@@ -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