[openssl-commits] [openssl] OpenSSL_1_1_0-stable update

[Richard Levitte]

The branch OpenSSL_1_1_0-stable has been updated
       via  2717f2b7eb845594271a94969767af2c4521e004 (commit)
      from  0aa0e13a6a367dc27bfa59bd2ab1e90645c3158b (commit)


- Log -----------------------------------------------------------------
commit 2717f2b7eb845594271a94969767af2c4521e004
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Dec 12 02:05:38 2017 +0100

    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 <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4913)
    
    (cherry picked from commit aef84bb4efbddfd95d042f3f5f1d362ed7d4faeb)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/err.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/crypto/err/err.c b/crypto/err/err.c
index 84eabc1..c439928 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) {