NULL pointer dereference - ASN1_STRING_get0_data() needs NULL checks

[dqp10515]

Version: 28f9147

Description: The function ASN1_STRING_get0_data() can return NULL, so NULL checks are needed in: mqtt_protocol/mosquitto/src/handle_connect.c line 806 and mqtt_protocol/mosquitto/src/security_default.c line 1205

When X509_STORE_CTX_get_ex_data() returns NULL, the runtime ASAN Log shows:

./08-ssl-hup-disconnect.py
FAIL: unable to start broker: 1760149307: Warning: Unable to drop privileges to 'mosquitto' because this user does not exist. Trying 'nobody' instead.
1760149307: mosquitto version 2.0.22 starting
1760149307: Config loaded from 08-ssl-hup-disconnect.conf.
1760149307: Warning: File 08-ssl-hup-disconnect.pwfile has world readable permissions. Future versions will refuse to load this file.
To fix this, use `chmod 0700 08-ssl-hup-disconnect.pwfile`.
1760149307: Warning: File 08-ssl-hup-disconnect.pwfile owner is not nobody. Future versions will refuse to load this file.To fix this, use `chown nobody 08-ssl-hup-disconnect.pwfile`.
1760149307: Warning: File 08-ssl-hup-disconnect.pwfile group is not nogroup. Future versions will refuse to load this file.
1760149307: Opening ipv4 listen socket on port 1888.
1760149307: Opening ipv6 listen socket on port 1888.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==52875==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f1274a35aef bp 0x000000000000 sp 0x7ffeab622c50 T0)
==52875==The signal is caused by a READ memory access.
==52875==Hint: address points to the zero page.
==52875==WARNING: invalid path to external symbolizer!
==52875==WARNING: Failed to use and restart external symbolizer!
    #0 0x7f1274a35aef  (openssl/libcrypto.so.1.1+0x94aef)
    #1 0x7f1274a3d8c7  (openssl/libcrypto.so.1.1+0x9c8c7)
    #2 0x7f1274a3fd90  (openssl/libcrypto.so.1.1+0x9ed90)
    #3 0x7f1274a406fb  (openssl/libcrypto.so.1.1+0x9f6fb)
    #4 0x7f1274bc6fd4  (openssl/libcrypto.so.1.1+0x225fd4)
    #5 0x7f1274bca90f  (openssl/libcrypto.so.1.1+0x22990f)
    #6 0x7f1274bcb116  (openssl/libcrypto.so.1.1+0x22a116)
    #7 0x7f1274bcb2f6  (openssl/libcrypto.so.1.1+0x22a2f6)
    #8 0x7f1274cfb974  (openssl/libssl.so.1.1+0x6c974)
    #9 0x7f1274cd34bf  (openssl/libssl.so.1.1+0x444bf)
    #10 0x7f1274cd35f6  (openssl/libssl.so.1.1+0x445f6)
    #11 0x5ee8c9  (mosquitto/src/mosquitto+0x5ee8c9)
    #12 0x5efe2d  (mosquitto/src/mosquitto+0x5efe2d)
    #13 0x5f0d0b  (mosquitto/src/mosquitto+0x5f0d0b)
    #14 0x4d2367  (mosquitto/src/mosquitto+0x4d2367)
    #15 0x4d0929  (mosquitto/src/mosquitto+0x4d0929)
    #16 0x4ce8c8  (mosquitto/src/mosquitto+0x4ce8c8)
    #17 0x7f12747960b2  (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #18 0x41fd8d  (mosquitto/src/mosquitto+0x41fd8d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (openssl/libcrypto.so.1.1+0x94aef) 
==52875==ABORTING

[ralight]

Can you please provide an example of how to reproduce this please?

[dqp10515]

This bug was discovered through fault injection testing on the OpenSSL 1.1 library. By injecting faults to force ASN1_STRING_get0_data() to return NULL, this vulnerability can be reproduced.

Reproduction Steps:

1. Apply fault injection to make ASN1_STRING_get0_data() return NULL
2. Run the test case: ./08-ssl-hup-disconnect.py
3. The broker will crash

Root Cause:

Potential Location 1: mosquitto/src/handle_connect.c line 806-808

#if OPENSSL_VERSION_NUMBER < 0x10100000L

new_username = (const char *) ASN1_STRING_data(name_asn1);

#else

new_username = (const char *) ASN1_STRING_get0_data(name_asn1);  // Can return NULL

#endif

if(mosquitto_validate_utf8(new_username, (int)strlen(new_username))){  // CRASH: strlen(NULL)

Potential Location 2: mosquitto/src/security_default.c line 1205-1214

#if OPENSSL_VERSION_NUMBER < 0x10100000L

context->username = mosquitto__strdup((char *) ASN1_STRING_data(name_asn1));

#else

context->username = mosquitto__strdup((char *) ASN1_STRING_get0_data(name_asn1));  // Can return NULL

#endif

if(!context->username){

Then at line 1214:

if ((size_t)ASN1_STRING_length(name_asn1) != strlen(context->username)) {  // Potential issue if strdup(NULL) succeeds

ASN1_STRING_get0_data() can return NULL in these scenarios:

1. Invalid ASN1_STRING structure: When the internal data pointer is not initialized
2. Memory allocation failures: In low-memory conditions
3. Corrupted certificate data: When certificate parsing encounters invalid data
4. Empty string fields: In some edge cases with empty certificate fields

[ralight]

Thanks for the detailed reply.

I don't really consider a fault injection to be a valid reproduction here. What I mean is that I don't believe that openssl will allow a certificate with an empty or invalid commonName value to be passed to user code. It still needs fixing, however unless you can reproduce it without a fault injection I do not believe it is able to be triggered in practice.

[dqp10515]

Thank you for the quick fix and analysis.