diff options
author | Todd Short <tshort@akamai.com> | 2017-05-11 21:48:10 +0200 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2017-05-11 22:35:21 +0200 |
commit | 7031ddac94d0ae616d1b0670263a9265ce672cd2 (patch) | |
tree | 291ee3feddc4a98ea47522b9ae94069821759d42 /test/secmemtest.c | |
parent | Clean away needless VMS check (diff) | |
download | openssl-7031ddac94d0ae616d1b0670263a9265ce672cd2.tar.xz openssl-7031ddac94d0ae616d1b0670263a9265ce672cd2.zip |
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 <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3449)
Diffstat (limited to '')
-rw-r--r-- | test/secmemtest.c | 21 |
1 files changed, 21 insertions, 0 deletions
diff --git a/test/secmemtest.c b/test/secmemtest.c index 3244d06b12..c92db50ace 100644 --- a/test/secmemtest.c +++ b/test/secmemtest.c @@ -61,6 +61,27 @@ static int test_sec_mem(void) || !TEST_true(CRYPTO_secure_malloc_done()) || !TEST_false(CRYPTO_secure_malloc_initialized())) goto end; + + TEST_info("Possible infinite loop: allocate more than available"); + if (!TEST_true(CRYPTO_secure_malloc_init(32768, 16))) + goto end; + TEST_ptr_null(OPENSSL_secure_malloc((size_t)-1)); + TEST_true(CRYPTO_secure_malloc_done()); + + TEST_info("Possible infinite loop: small arena"); + if (!TEST_false(CRYPTO_secure_malloc_init(16, 16))) + goto end; + TEST_false(CRYPTO_secure_malloc_initialized()); + TEST_ptr_null(OPENSSL_secure_malloc((size_t)-1)); + TEST_true(CRYPTO_secure_malloc_done()); + + if (sizeof(size_t) > 4) { + TEST_info("Possible infinite loop: 1<<31 limit"); + if (!TEST_true(CRYPTO_secure_malloc_init((size_t)1<<34, (size_t)1<<4) != 0)) + goto end; + TEST_true(CRYPTO_secure_malloc_done()); + } + /* this can complete - it was not really secure */ testresult = 1; end: |