Lets take a look at the vulnerable code:
if (s->servername_done == 0) {
switch (servname_type) {
case TLSEXT_NAMETYPE_host_name:
if (s->session->tlsext_hostname == NULL) {
if (len > TLSEXT_MAXLEN_host_name ||
((s->session->tlsext_hostname = OPENSSL_malloc(len + 1)) == NULL)) {
*al = TLS1_AD_UNRECOGNIZED_NAME;
return 0;
}
memcpy(s->session->tlsext_hostname, sdata, len);
s->session->tlsext_hostname[len] = '\0';
}
}
}Here if tlsext_hostname is set to NULL, it will allocate len+1 bytes, then memcpy into that buffer with a size of len. Nothing out of the ordinary, But there were some oversights here that take a little digging to see.
OpenSSL is not thread-safe, it wasn't designed that way. You can force openssl to be thread safe, but it's obvious that the code to do this was wedged in there at the last minute.
Here is how we initialize OpenSSL to be thread-safe:
pthread_mutex_t *thread_locks;
unsigned long
openssl_thread_id(void) {
return (unsigned long) pthread_self();
}
void
openssl_thread_lock( int mode, int lock_id, const char* file, int line ) {
if ( mode & CRYPTO_LOCK )
pthread_mutex_lock( &thread_locks[lock_id] );
else
pthread_mutex_unlock( &thread_locks[lock_id] );
}
void init_ssl_lock(void) {
int num_thread_locks = CRYPTO_num_locks();
thread_locks = calloc( sizeof( pthread_mutex_t ), num_thread_locks );
for (i = 0; i < num_thread_locks; i++)
{
if ( pthread_mutex_init( &thread_locks[i], NULL ) )
{
fprintf(stderr, "Unable to create mutex\n");
exit(80);
}
}
CRYPTO_set_locking_callback( &openssl_thread_lock );
CRYPTO_set_id_callback( &openssl_thread_id );
}We can usually assume this would cover any type of potential race-conditions, but in this case, one was overlooked.
There are several functions that add, fetch, and remove entries from the session cache: ssl_get_new_session(), ssl_get_prev_session(), SSL_CTX_add_session(), SSL_CTX_remove_session(). These all call CRYPTO_w_lock()/CRYPTO_w_unlock() for any type of data modifications. Yay!
Here is the catch: ssl->session is shared across multiple threads, modified by the functions above. But in the function ssl_parse_clienthello_tlsext(), the value of s->session->tlsext_hostname is checked for a NULL value without any locking.
There is a possibility that (if the value of tlsext_hostname is NULL) that two threads enter the same block of code at the same time, one overwiting the others pointer to tlsext_hostname. The outcome of the flow would look something like this:
[ Thread-A ] [ Thread-B ]
if (tlsext_hostname == NULL) | |
| | if (tlsext_hostname == NULL)
tlsext_hostname = malloc(256); | |
| | tlsext_hostname = malloc(1);
memcpy(tlsext_hostname, buf, 255); | |
| memcpy(tlsext_hostname, buf, 1);
What happens here is Thread-A sees that tlsext_hostname is NULL. Thread-A then allocates a user controlled length (note: max of 256), finally doing memcpy of the user controlled data with a size of 255 (leaving room for the trailing '\0'). But in this case, between the time Thread-A allocated the buffer and the memcpy, Thread-B had allocated a much smaller amount of memory into the tlsext_hostname variable.
If executed properly, Thread-A would overwrite the heap by 254 bytes (255 - 1)
So the fix was basically to check the value of s->hit before doing anything stupid. Why?
int ssl3_get_client_hello(SSL *s) {
/* bunch of crap here */
/* remember, ssl_get_prev_session properly locks s->session */
i = ssl_get_prev_session(s, p, j, d + n);
if (i == 1) { /* previous session */
s->hit=1;
}
}SSL *s is allocated on the heap thus thread safe. If ssl_get_prev_session() (which does proper locking) returns 1, s->hit is set, and then used later on inside ssl_parse_clienthello_tlsext to determine if tlsext_hostname is already allocated.