possible NULL dereference in context.c (openssl-3.0.0) #17312

easylyou · 2021-12-20T08:46:14Z

Hi!
We are using improved fuzzing technology with fault injection. These are possible NULL dereference issues.

Different from traditional fuzzing, fault injection can test error-handling codes while the errors happen in some extreme conditions. (such as malloc() return NULL when out of memory). So it is hard to reproduce without our fault injection environment. I try to analysis the related codes to find the cause but not sure, needing your helps. I'm glad to provide more details if you need.

Analysis:

// in crypto/context.c:346
346 void *ossl_lib_ctx_get_data( ...
348 {
        // `ossl_lib_ctx_get_concrete()` never return NULL even if `context_init()` fails, but `default_context_int` instead.
        // see below.
352     ctx = ossl_lib_ctx_get_concrete(ctx);
353     if (ctx == NULL)
354         return NULL;
        // `ctx->lock == NULL`, NULL point dereference.
356     if (!CRYPTO_THREAD_read_lock(ctx->lock)) 
357         return NULL;

418 }

// in crypto/context.c:169
169 static OSSL_LIB_CTX *get_default_context(void)
170 {
171     OSSL_LIB_CTX *current_defctx = get_thread_default_context();
173     if (current_defctx == NULL)
            // return `default_context_int`.
174         current_defctx = &default_context_int;
175     return current_defctx;
176 }

error logs:

==2096623==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000018 (pc 0x7f79dcc373f0 bp 0x7ffe406a7b30 sp 0x7ffe406a7ad0 T0)
==2096623==The signal is caused by a READ memory access.
==2096623==Hint: address points to the zero page.
    #0 0x7f79dcc373f0 in __pthread_rwlock_rdlock_full /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_rwlock_common.c:299:7
    #1 0x7f79dcc373f0 in pthread_rwlock_rdlock /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_rwlock_rdlock.c:27:16
    #2 0xb1f88e in CRYPTO_THREAD_read_lock /home/r1/openssl/openssl-3.0.0/build/../crypto/threads_pthread.c:85:9
    #3 0xab473c in ossl_lib_ctx_get_data /home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:356:10
    #4 0xb07282 in get_provider_store /home/r1/openssl/openssl-3.0.0/build/../crypto/provider_core.c:334:13
    #5 0xb07460 in ossl_provider_info_add_to_store /home/r1/openssl/openssl-3.0.0/build/../crypto/provider_core.c:360:39
    #6 0xb0377e in OSSL_PROVIDER_add_builtin /home/r1/openssl/openssl-3.0.0/build/../crypto/provider.c:125:10
    #7 0x4c55b2 in FuzzerSetRand /home/r1/openssl/openssl-3.0.0/build/../fuzz/fuzz_rand.c:157:10
    #8 0x4c461e in FuzzerInitialize /home/r1/openssl/openssl-3.0.0/build/../fuzz/client.c:43:5
    #9 0x4c6879 in main /home/r1/openssl/openssl-3.0.0/build/../fuzz/test-corpus.c:64:5
    #10 0x7f79dc8e90b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #11 0x41c6ad in _start (/home/r1/openssl/bin/client-test+0x41c6ad)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_rwlock_common.c:299:7 in __pthread_rwlock_rdlock_full
==2096623==ABORTING

The text was updated successfully, but these errors were encountered:

paulidale · 2021-12-20T09:36:26Z

I don't see how default_context_int can have a NULL lock field. The initialisation

openssl/crypto/context.c

Lines 150 to 154 in 0d4c523

    
           DEFINE_RUN_ONCE_STATIC(default_context_do_init) 
        
           { 
        
               return CRYPTO_THREAD_init_local(&default_context_thread_local, NULL) 
        
                   && context_init(&default_context_int); 
        
           }

indicates failure if the lock isn't created. I only tracked the calls one step back, so I guess something might be ignoring a failure return.

Do you know how your fuzzer managed to get here?

t8m · 2021-12-20T09:50:42Z

I suppose when CRYPTO_THREAD_init_local() or context_init() fails the default_context_int will be uninitialized, i.e. all zeros.

Not sure we have to guard against such kind of failures. Similarly for #17313

t8m · 2021-12-20T09:52:13Z

I.e., should we really guard against situation when initialization of global data that is "always expected to be in there" fails?

easylyou · 2021-12-20T10:07:37Z

I suppose when CRYPTO_THREAD_init_local() or context_init() fails the default_context_int will be uninitialized, i.e. all zeros.

Not sure we have to guard against such kind of failures. Similarly for #17313

Your supposition is right! Here is the injection log.
Only one inject here. The log is the backtrack of the inject point.

===================FAULT INJECT===================
faultinject: error actived 1! name: malloc, file: ../crypto/mem.c, line: 184
??:?
/home/r1/src_git/mfuzz-project/wrapper/mfuzzrt/faultinject.c:45
/home/r1/openssl/openssl-3.0.0/build/../crypto/mem.c:184
/home/r1/openssl/openssl-3.0.0/build/../crypto/mem.c:191
/home/r1/openssl/openssl-3.0.0/build/../crypto/stack/stack.c:221
/home/r1/openssl/openssl-3.0.0/build/../crypto/stack/stack.c:130
/home/r1/openssl/openssl-3.0.0/build/../crypto/ex_data.c:470
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:312
/home/r1/openssl/openssl-3.0.0/build/../crypto/ex_data.c:458
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:407
/home/r1/openssl/openssl-3.0.0/build/../crypto/property/property_string.c:214
/home/r1/openssl/openssl-3.0.0/build/../crypto/property/property_parse.c:546
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:102
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:153   <---------   context_init(&default_context_int)      
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:150
/lib/x86_64-linux-gnu/libpthread.so.0(+0x1247f)[0x7faef2a2647f]
/home/r1/openssl/openssl-3.0.0/build/../crypto/threads_pthread.c:144
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:163
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:171
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:274
/home/r1/openssl/openssl-3.0.0/build/../crypto/context.c:352
/home/r1/openssl/openssl-3.0.0/build/../crypto/provider_core.c:334
/home/r1/openssl/openssl-3.0.0/build/../crypto/provider_core.c:360
/home/r1/openssl/openssl-3.0.0/build/../crypto/provider.c:125
/home/r1/openssl/openssl-3.0.0/build/../fuzz/fuzz_rand.c:157
/home/r1/openssl/openssl-3.0.0/build/../fuzz/client.c:43
/home/r1/openssl/openssl-3.0.0/build/../fuzz/test-corpus.c:64
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7faef26d40b3]
??:?
==================================================

161 static OSSL_LIB_CTX *get_thread_default_context(void)
162 {
           // `default_context_init` fail, return NULL.
163     if (!RUN_ONCE(&default_context_init, default_context_do_init))
164         return NULL;
165 
166     return CRYPTO_THREAD_get_local(&default_context_thread_local);
167 }
168 
169 static OSSL_LIB_CTX *get_default_context(void)
170 {
           // `default_context_init` fail, `current_defctx = NULL`.
171     OSSL_LIB_CTX *current_defctx = get_thread_default_context();
172 
173     if (current_defctx == NULL)
              // Point to `default_context_int` again. In fact it is all zero.
174         current_defctx = &default_context_int;
175     return current_defctx;
176 }

easylyou · 2021-12-20T10:13:23Z

I.e., should we really guard against situation when initialization of global data that is "always expected to be in there" fails?

I agree with you. Ignoring some errors and let it crash is also a good solution in some situations.
In fact I'm not familiar with openssl. In usual, if a software may request global resources such as shared-memory, global lock or modifying filesystem and so on, it's better not to simply ignore.

paulidale · 2021-12-20T23:21:44Z

I.e., should we really guard against situation when initialization of global data that is "always expected to be in there" fails?

Shouldn't we detect that the initialisation failed and return an error? If an application then continues using us, crash is acceptable, although more errors would be better IMO.

artuzap · 2022-11-14T15:38:56Z

A real life example from a program using libldap 2.6.2 and OpenSSL 3.0.1 (CentOS 9).
At first OPENSSL_cleanup() is called (registered with atexit()):

0  ossl_lib_ctx_default_deinit () at crypto/context.c:196
1  OPENSSL_cleanup () at crypto/init.c:424
2  OPENSSL_cleanup () at crypto/init.c:338
3  0x00007ffff7873475 in __run_exit_handlers (status=0, listp=0x7ffff7a11658 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:113
4  0x00007ffff78735f0 in __GI_exit (status=<optimized out>) at exit.c:143
5  0x00007ffff785be57 in __libc_start_call_main (main=main@entry=0x55555556aa20 <main>, argc=argc@entry=4, argv=argv@entry=0x7fffffffe2c8) at ../sysdeps/nptl/libc_start_call_main.h:74
6  0x00007ffff785befc in __libc_start_main_impl (main=0x55555556aa20 <main>, argc=4, argv=0x7fffffffe2c8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe2b8) at ../csu/libc-start.c:409
7  0x000055555556b575 in _start ()

Then SSL_CTX_free() is called indirectly by ldap_int_destroy_global_options() (not shown on stacktrace, registered with __attribute__ ((destructor))):

Program received signal SIGSEGV, Segmentation fault.

0  ___pthread_rwlock_rdlock (rwlock=0x0) at pthread_rwlock_rdlock.c:27
1  0x00007ffff7c92f3d in CRYPTO_THREAD_read_lock (lock=<optimized out>) at crypto/threads_pthread.c:85
2  0x00007ffff7c8b126 in ossl_lib_ctx_get_data (ctx=0x7ffff7eff540 <default_context_int.lto_priv>, index=1, meth=0x7ffff7eb8a00 <provider_store_method.lto_priv>) at crypto/context.c:398
3  0x00007ffff7c98bea in get_provider_store (libctx=<optimized out>) at crypto/provider_core.c:334
4  ossl_provider_deregister_child_cb (handle=0x5555555ed620) at crypto/provider_core.c:1752
5  0x00007ffff7c8bf2f in ossl_provider_deinit_child (ctx=0x5555555d2650) at crypto/provider_child.c:279
6  OSSL_LIB_CTX_free (ctx=0x5555555d2650) at crypto/context.c:283
7  OSSL_LIB_CTX_free (ctx=0x5555555d2650) at crypto/context.c:276
8  0x00007ffff7634af6 in legacy_teardown (provctx=0x5555555ee9f0) at providers/legacyprov.c:168
9  0x00007ffff7c9901b in ossl_provider_teardown (prov=0x5555555ed620) at crypto/provider_core.c:1477
10 ossl_provider_free (prov=0x5555555ed620) at crypto/provider_core.c:683
11 0x00007ffff7c63956 in ossl_provider_free (prov=<optimized out>) at crypto/provider_core.c:668
12 evp_cipher_free_int (cipher=0x555555916c10) at crypto/evp/evp_enc.c:1632
13 EVP_CIPHER_free (cipher=0x555555916c10) at crypto/evp/evp_enc.c:1647
14 0x00007ffff7a6bc1d in ssl_evp_cipher_free (cipher=0x555555916c10) at ssl/ssl_lib.c:5925
15 ssl_evp_cipher_free (cipher=0x555555916c10) at ssl/ssl_lib.c:5915
16 SSL_CTX_free (a=0x555555ec1020) at ssl/ssl_lib.c:3455
17 SSL_CTX_free (a=0x555555ec1020) at ssl/ssl_lib.c:3392
18 0x00007fffe95edb89 in ldap_int_tls_destroy (lo=0x7fffe9616000 <ldap_int_global_options>) at /usr/src/debug/openldap-2.6.2-1.el9_0.x86_64/openldap-2.6.2/libraries/libldap/tls2.c:104
19 0x00007ffff7fd100b in _dl_fini () at dl-fini.c:138
20 0x00007ffff7873475 in __run_exit_handlers (status=0, listp=0x7ffff7a11658 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:113
21 0x00007ffff78735f0 in __GI_exit (status=<optimized out>) at exit.c:143
22 0x00007ffff785be57 in __libc_start_call_main (main=main@entry=0x55555556aa20 <main>, argc=argc@entry=4, argv=argv@entry=0x7fffffffe2b8) at ../sysdeps/nptl/libc_start_call_main.h:74
23 0x00007ffff785befc in __libc_start_main_impl (main=0x55555556aa20 <main>, argc=4, argv=0x7fffffffe2b8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe2a8) at ../csu/libc-start.c:409
24 0x000055555556b575 in _start ()

Segfault happens only when legacy providers are loaded. But the point is that even when there is no segfault, the order of deinitialization is wrong and it is left undetected.

rcmcdonald91 · 2023-10-18T20:22:44Z

I am seeing this exact issue with FreeBSD pkg using libcurl as a fetch backend on FreeBSD CURRENT (OpenSSL 3.0.11).

rcmcdonald91 · 2023-10-19T13:42:13Z

So, it seems this issue is caused by libcurl not initializing OpenSSL with OPENSSL_INIT_NOATEXIT. A quick search revealed that other OpenSSL consumers have experienced issues with atexit ordering and cleanup over the years. So, I'm not sure there is anything to really fix on OpenSSL's side.

Library consumers of OpenSSL should probably by default use OPENSSL_INIT_NOATEXIT, especially as they have no idea the context of the application....or use some other mechanism to ensure correct deinitialization.

easylyou added the issue: bug report The issue was opened to report a bug label Dec 20, 2021

paulidale added branch: 3.0 Merge to openssl-3.0 branch branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug and removed issue: bug report The issue was opened to report a bug labels Dec 20, 2021

This was referenced Oct 18, 2023

segfault during pkg_shutdown when using openssl legacy providers and pkg+https scheme freebsd/pkg#2191

Open

openssl: use OPENSSL_INIT_NO_ATEXIT curl/curl#12153

Closed

t8m added branch: 3.1 Merge to openssl-3.1 help wanted labels Oct 19, 2023

t8m added the branch: 3.2 Merge to openssl-3.2 label Oct 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

possible NULL dereference in context.c (openssl-3.0.0) #17312

possible NULL dereference in context.c (openssl-3.0.0) #17312

easylyou commented Dec 20, 2021

paulidale commented Dec 20, 2021

t8m commented Dec 20, 2021

t8m commented Dec 20, 2021

easylyou commented Dec 20, 2021

easylyou commented Dec 20, 2021

paulidale commented Dec 20, 2021

artuzap commented Nov 14, 2022

rcmcdonald91 commented Oct 18, 2023

rcmcdonald91 commented Oct 19, 2023 •

edited

Loading

possible NULL dereference in context.c (openssl-3.0.0) #17312

possible NULL dereference in context.c (openssl-3.0.0) #17312

Comments

easylyou commented Dec 20, 2021

paulidale commented Dec 20, 2021

t8m commented Dec 20, 2021

t8m commented Dec 20, 2021

easylyou commented Dec 20, 2021

easylyou commented Dec 20, 2021

paulidale commented Dec 20, 2021

artuzap commented Nov 14, 2022

rcmcdonald91 commented Oct 18, 2023

rcmcdonald91 commented Oct 19, 2023 • edited Loading

rcmcdonald91 commented Oct 19, 2023 •

edited

Loading