From 7031ddac94d0ae616d1b0670263a9265ce672cd2 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 11 May 2017 15:48:10 -0400 Subject: Fix infinite loops in secure memory allocation. Issue 1: sh.bittable_size is a size_t but i is and int, which can result in freelist == -1 if sh.bittable_size exceeds an int. This seems to result in an OPENSSL_assert due to invalid allocation size, so maybe that is "ok." Worse, if sh.bittable_size is exactly 1<<31, then this becomes an infinite loop (because 1<<31 is a negative int, so it can be shifted right forever and sticks at -1). Issue 2: CRYPTO_secure_malloc_init() sets secure_mem_initialized=1 even when sh_init() returns 0. If sh_init() fails, we end up with secure_mem_initialized=1 but sh.minsize=0. If you then call secure_malloc(), which then calls, sh_malloc(), this then enters an infite loop since 0 << anything will never be larger than size. Issue 3: That same sh_malloc loop will loop forever for a size greater than size_t/2 because i will proceed (assuming sh.minsize=16): i=16, 32, 64, ..., size_t/8, size_t/4, size_t/2, 0, 0, 0, 0, .... This sequence will never be larger than "size". Reviewed-by: Rich Salz Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/3449) --- crypto/mem_sec.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'crypto') diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c index 351dec43bc..774b696057 100644 --- a/crypto/mem_sec.c +++ b/crypto/mem_sec.c @@ -73,8 +73,12 @@ int CRYPTO_secure_malloc_init(size_t size, int minsize) sec_malloc_lock = CRYPTO_THREAD_lock_new(); if (sec_malloc_lock == NULL) return 0; - ret = sh_init(size, minsize); - secure_mem_initialized = 1; + if ((ret = sh_init(size, minsize)) != 0) { + secure_mem_initialized = 1; + } else { + CRYPTO_THREAD_lock_free(sec_malloc_lock); + sec_malloc_lock = NULL; + } } return ret; @@ -90,6 +94,7 @@ int CRYPTO_secure_malloc_done() sh_done(); secure_mem_initialized = 0; CRYPTO_THREAD_lock_free(sec_malloc_lock); + sec_malloc_lock = NULL; return 1; } #endif /* IMPLEMENTED */ @@ -341,7 +346,8 @@ static void sh_remove_from_list(char *ptr) static int sh_init(size_t size, int minsize) { - int i, ret; + int ret; + size_t i; size_t pgsize; size_t aligned; @@ -498,6 +504,9 @@ static void *sh_malloc(size_t size) size_t i; char *chunk; + if (size > sh.arena_size) + return NULL; + list = sh.freelist_size - 1; for (i = sh.minsize; i < size; i <<= 1) list--; -- cgit v1.2.3