From 2717f2b7eb845594271a94969767af2c4521e004 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 12 Dec 2017 02:05:38 +0100 Subject: [PATCH] Fix leak in ERR_get_state() when OPENSSL_init_crypto() isn't called yet If OPENSSL_init_crypto() hasn't been called yet when ERR_get_state() is called, it need to be called early, so the base initialization is done. On some platforms (those who support DSO functionality and don't define OPENSSL_USE_NODELETE), that includes a call of ERR_set_mark(), which calls this function again. Furthermore, we know that ossl_init_thread_start(), which is called later in ERR_get_state(), calls OPENSSL_init_crypto(0, NULL), except that's too late. Here's what happens without an early call of OPENSSL_init_crypto(): => ERR_get_state(): => CRYPTO_THREAD_get_local(): <= NULL; # no state is found, so it gets allocated. => ossl_init_thread_start(): => OPENSSL_init_crypto(): # Here, base_inited is set to 1 # before ERR_set_mark() call => ERR_set_mark(): => ERR_get_state(): => CRYPTO_THREAD_get_local(): <= NULL; # no state is found, so it gets allocated!!!!! => ossl_init_thread_start(): => OPENSSL_init_crypto(): # base_inited is 1, # so no more init to be done <= 1 <= => CRYPTO_thread_set_local(): <= <= <= <= 1 <= => CRYPTO_thread_set_local() # previous value removed! <= Result: double allocation, and we have a leak. By calling the base OPENSSL_init_crypto() early, we get this instead: => ERR_get_state(): => OPENSSL_init_crypto(): # Here, base_inited is set to 1 # before ERR_set_mark() call => ERR_set_mark(): => ERR_get_state(): => OPENSSL_init_crypto(): # base_inited is 1, # so no more init to be done <= 1 => CRYPTO_THREAD_get_local(): <= NULL; # no state is found, so it gets allocated # let's assume we got 0xDEADBEEF => ossl_init_thread_start(): => OPENSSL_init_crypto(): # base_inited is 1, # so no more init to be done <= 1 <= 1 => CRYPTO_thread_set_local(): <= <= <= <= 1 => CRYPTO_THREAD_get_local(): <= 0xDEADBEEF <= 0xDEADBEEF Result: no leak. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/4913) (cherry picked from commit aef84bb4efbddfd95d042f3f5f1d362ed7d4faeb) --- crypto/err/err.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crypto/err/err.c b/crypto/err/err.c index 84eabc1772..c4399285fe 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -658,6 +658,14 @@ ERR_STATE *ERR_get_state(void) if (!RUN_ONCE(&err_init, err_do_init)) return NULL; + /* + * If base OPENSSL_init_crypto() hasn't been called yet, be sure to call + * it now to avoid state to be doubly allocated and thereby leak memory. + * Needed on any platform that doesn't define OPENSSL_USE_NODELETE. + */ + if (!OPENSSL_init_crypto(0, NULL)) + return NULL; + state = CRYPTO_THREAD_get_local(&err_thread_local); if (state == NULL) { -- 2.25.1