openssl: version 1.1.1L crashes with null pointer access in ERR_load_strings()

Hi all, version: 1.1.1L Built with option: no-err Callstack: pthread_rwlock_trywrlock(NULL) pthread_rwlock_timedwrlock(NULL) CRYPTO_THREAD_write_lock(NULL) ERR_load_strings() xmlSecOpenSSLInit()

Original openssl code:

int ERR_load_ERR_strings(void)
{
**#ifndef OPENSSL_NO_ERR**
    if (!RUN_ONCE(&err_string_init, do_err_strings_init))
        return 0;

    err_load_strings(ERR_str_libraries);
    err_load_strings(ERR_str_reasons);
    err_patch(ERR_LIB_SYS, ERR_str_functs);
    err_load_strings(ERR_str_functs);
    build_SYS_str_reasons();
#endif
    return 1;
}

If changed to:

int ERR_load_ERR_strings(void)
{
    if (!RUN_ONCE(&err_string_init, do_err_strings_init))
        return 0;

**#ifndef OPENSSL_NO_ERR**
    err_load_strings(ERR_str_libraries);
    err_load_strings(ERR_str_reasons);
    err_patch(ERR_LIB_SYS, ERR_str_functs);
    err_load_strings(ERR_str_functs);
    build_SYS_str_reasons();
#endif
    return 1;
}

Crash does not happen. Is this modification a valid fix?

Thanks, Yusshi

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 15 (10 by maintainers)

Commits related to this issue

err: fix crash in ERR_load_strings() when configured with no-err This commit removes the entire initialization and cleanup of the error string hash table (`int_error_hash`) if `no-err` is configured.... — committed to mspncp/openssl by mspncp 2 years ago
err: fix crash in ERR_load_strings() when configured with no-err This commit removes the entire initialization and cleanup of the error string hash table (`int_error_hash`) if `no-err` is configured.... — committed to mspncp/openssl by mspncp 2 years ago
err: fix crash in ERR_load_strings() when configured with no-err This commit removes the entire initialization and cleanup of the error string hash table (`int_error_hash`) if `no-err` is configured.... — committed to openssl/openssl by mspncp 2 years ago
err: fix crash in ERR_load_strings() when configured with no-err This commit removes the entire initialization and cleanup of the error string hash table (`int_error_hash`) if `no-err` is configured.... — committed to openssl/openssl by mspncp 2 years ago
err: fix crash in ERR_load_strings() when configured with no-err This commit removes the entire initialization and cleanup of the error string hash table (`int_error_hash`) if `no-err` is configured.... — committed to openssl/openssl by mspncp 2 years ago
Upstream 1.1.1s (#407) * Update copyright year Reviewed-by: Richard Levitte <levitte@openssl.org> * Run make update Reviewed-by: Richard Levitte <levitte@openssl.org> * Prepare for 1.1.1l... — committed to open-quantum-safe/openssl by baentsch 2 years ago
Upgrade to upstream 1.1.1t (#430) * VMS: Fix misspelt type '__int64', not 'int64_t' Ref: commit 2e5cdbc18a1a26bfc817070a52689886fa0669c2 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed... — committed to open-quantum-safe/openssl by baentsch a year ago
Upgrade to upstream 1.1.1t (#430) * VMS: Fix misspelt type '__int64', not 'int64_t' Ref: commit 2e5cdbc18a1a26bfc817070a52689886fa0669c2 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed... — committed to mamckee/openssl by baentsch a year ago

Most upvoted comments

@mspncp , Sure we can test the patches. Will update when test is done.

yusshi on Mar 29, 2022

Please correct me if I am wrong. My understanding for no-err is just not to populate the internal error lib/strings of openssl code, but still support external code, such as xmlsec1/curl, etc, to register error lib/strings. So guard all the lines in ERR_load_ERR_strings() may not serve the purpose.

The documentation is not very verbose about this option. The only location I could find is in the INSTALL file:

no-err

Don’t compile in any error strings.

Given the fuzziness of the explanation, one could indeed interpret it in the way you understood it.

But if you were right, this would be the most ridiculous OpenSSL compile time option I’ve encountered so far: the only reasonable justification for the no-err option is to save as much precious space as possible in a limited environment. What sense does it make to apply all the effort to remove all OpenSSL error strings, just to keep third party error strings afterwards?

If people configure OpenSSL without support for error strings, then that’s exactly what they should get: no error strings, only error codes, basta.

mspncp on Mar 29, 2022

ERR_get_next_error_library() should actually be a no-op in the no-err case, too. And err_string_lock should not be defined at all. See my other https://github.com/openssl/openssl/pull/17975#issuecomment-1080529490.

mspncp on Mar 28, 2022