[openssl.org #1743] crasher due to lack of threadsafety on names_lh

John Wilkinson via RT Fri, 12 Sep 2008 02:52:27 -0700

diff -ru openssl-0.9.8h/crypto/cryptlib.c 
openssl-0.9.8h-patched/crypto/cryptlib.c
--- openssl-0.9.8h/crypto/cryptlib.c    2007-09-06 12:43:46.000000000 +0000
+++ openssl-0.9.8h-patched/crypto/cryptlib.c    2008-09-11 18:21:33.000000000 
+0000
@@ -166,7 +166,8 @@
        "ec_pre_comp",
        "store",
        "comp",
-#if CRYPTO_NUM_LOCKS != 39
+       "names_lh",
+#if CRYPTO_NUM_LOCKS != 40
 # error "Inconsistency between crypto.h and cryptlib.c"
 #endif
        };
diff -ru openssl-0.9.8h/crypto/crypto.h openssl-0.9.8h-patched/crypto/crypto.h
--- openssl-0.9.8h/crypto/crypto.h      2005-05-08 19:54:33.000000000 +0000
+++ openssl-0.9.8h-patched/crypto/crypto.h      2008-09-11 18:20:59.000000000 
+0000
@@ -219,7 +219,8 @@
 #define CRYPTO_LOCK_EC_PRE_COMP                36
 #define CRYPTO_LOCK_STORE              37
 #define CRYPTO_LOCK_COMP               38
-#define CRYPTO_NUM_LOCKS               39
+#define CRYPTO_LOCK_NAMES_LH           39
+#define CRYPTO_NUM_LOCKS               40
 
 #define CRYPTO_LOCK            1
 #define CRYPTO_UNLOCK          2
diff -ru openssl-0.9.8h/crypto/objects/o_names.c 
openssl-0.9.8h-patched/crypto/objects/o_names.c
--- openssl-0.9.8h/crypto/objects/o_names.c     2005-04-05 10:29:42.000000000 
+0000
+++ openssl-0.9.8h-patched/crypto/objects/o_names.c     2008-09-11 
18:35:12.000000000 +0000
@@ -48,11 +48,19 @@
 
 int OBJ_NAME_init(void)
        {
-       if (names_lh != NULL) return(1);
+       int ret;
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
+       if (names_lh != NULL)
+               {
+               CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
+               return(1);
+               }
        MemCheck_off();
        names_lh=lh_new(obj_name_hash, obj_name_cmp);
        MemCheck_on();
-       return(names_lh != NULL);
+       ret = (names_lh != NULL);
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
+       return(ret);
        }
 
 int OBJ_NAME_new_index(unsigned long (*hash_func)(const char *),
@@ -164,7 +172,9 @@
 
        for (;;)
        {
+       CRYPTO_r_lock(CRYPTO_LOCK_NAMES_LH);
                ret=(OBJ_NAME *)lh_retrieve(names_lh,&on);
+       CRYPTO_r_unlock(CRYPTO_LOCK_NAMES_LH);
                if (ret == NULL) return(NULL);
                if ((ret->alias) && !alias)
                        {
@@ -200,7 +210,9 @@
        onp->type=type;
        onp->data=data;
 
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
        ret=(OBJ_NAME *)lh_insert(names_lh,onp);
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
        if (ret != NULL)
                {
                /* free things */
@@ -217,7 +229,11 @@
                }
        else
                {
-               if (lh_error(names_lh))
+               int names_lh_error;
+       CRYPTO_r_lock(CRYPTO_LOCK_NAMES_LH);
+       names_lh_error = lh_error(names_lh);
+       CRYPTO_r_unlock(CRYPTO_LOCK_NAMES_LH);
+               if (names_lh_error)
                        {
                        /* ERROR */
                        return(0);
@@ -235,7 +251,9 @@
        type&= ~OBJ_NAME_ALIAS;
        on.name=name;
        on.type=type;
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
        ret=(OBJ_NAME *)lh_delete(names_lh,&on);
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
        if (ret != NULL)
                {
                /* free things */
@@ -278,7 +296,9 @@
        d.fn=fn;
        d.arg=arg;
 
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
        lh_doall_arg(names_lh,LHASH_DOALL_ARG_FN(do_all_fn),&d);
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
        }
 
 struct doall_sorted
@@ -313,7 +333,9 @@
        int n;
 
        d.type=type;
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
        d.names=OPENSSL_malloc(lh_num_items(names_lh)*sizeof *d.names);
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
        d.n=0;
        OBJ_NAME_do_all(type,do_all_sorted_fn,&d);
 
@@ -352,6 +374,9 @@
        if (names_lh == NULL) return;
 
        free_type=type;
+
+       CRYPTO_w_lock(CRYPTO_LOCK_NAMES_LH);
+
        down_load=names_lh->down_load;
        names_lh->down_load=0;
 
@@ -365,5 +390,6 @@
                }
        else
                names_lh->down_load=down_load;
-       }
 
+       CRYPTO_w_unlock(CRYPTO_LOCK_NAMES_LH);
+       }

OpenSSL version: openssl-0.9.8h
 
To reproduce more than one thread must call SSL_library_init or other
functions that modify names_lh at approximiately the same time.
 
The stack traces will look something like:
 
#0  0x0097aa52 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
#1  0x001503ae in *__GI_raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:67
#2  0x001518f8 in *__GI_abort () at ../sysdeps/generic/abort.c:88
#3  0x0017ecf9 in __libc_message (do_abort=2, fmt=0x220da0 "*** glibc
detected *** %s: 0x%s ***\n") at
../sysdeps/unix/sysv/linux/libc_fatal.c:145
#4  0x00183e1a in malloc_printerr (action=2, str=0x220e0c "double free
or corruption (fasttop)", ptr=0xb7401fb8) at malloc.c:5525
#5  0x001857d5 in _int_realloc (av=0xb7400010, oldmem=0xb7401fb8,
bytes=0) at malloc.c:4668
#6  0x00186684 in *__GI___libc_realloc (oldmem=0xb7401fb8, bytes=128) at
malloc.c:3487
#7  0x00186c9c in realloc_hook_ini (ptr=0xb7401fb8, sz=128,
caller=0x81cced6) at hooks.c:54
#8  0x001865d1 in *__GI___libc_realloc (oldmem=0xb7401fb8, bytes=128) at
malloc.c:3425
#9  0x081cced6 in default_realloc_ex (str=0xb7401fb8, num=128,
file=0x829df09 "lhash.c", line=342) at mem.c:86
#10 0x081cd4ff in CRYPTO_realloc (str=0xb7401fb8, num=128,
file=0x829df09 "lhash.c", line=342) at mem.c:331
#11 0x081e851a in expand (lh=0xb7401f50) at lhash.c:341
#12 0x081e81a0 in lh_insert (lh=0xb7401f50, data=0xb7402280) at
lhash.c:187
#13 0x081cfbd8 in OBJ_NAME_add (name=0x828ef27 "ssl2-md5", type=1,
data=0x828ef23 "MD5") at o_names.c:203
#14 0x081cc5a4 in SSL_library_init () at ssl_algs.c:100

See attached patch file for fix.
 
This doesn't appear to be an exploitable security vulnerability as far
as I can tell.  This bug appears to have existed as far back as 1999
based on looking at 
http://cvs.openssl.org/rlog?f=openssl/crypto/objects/o_names.c

OpenSSL version: openssl-0.9.8h

To reproduce more than one thread must call SSL_library_init or other functions that modify names_lh at approximiately the same time.

The stack traces will look something like:

#0 0x0097aa52 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
#1 0x001503ae in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67
#2 0x001518f8 in *__GI_abort () at ../sysdeps/generic/abort.c:88
#3 0x0017ecf9 in __libc_message (do_abort=2, fmt=0x220da0 "*** glibc detected *** %s: 0x%s ***\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:145
#4 0x00183e1a in malloc_printerr (action="" str=0x220e0c "double free or corruption (fasttop)", ptr=0xb7401fb8) at malloc.c:5525
#5 0x001857d5 in _int_realloc (av=0xb7400010, oldmem=0xb7401fb8, bytes=0) at malloc.c:4668
#6 0x00186684 in *__GI___libc_realloc (oldmem=0xb7401fb8, bytes=128) at malloc.c:3487
#7 0x00186c9c in realloc_hook_ini (ptr=0xb7401fb8, sz=128, caller=0x81cced6) at hooks.c:54
#8 0x001865d1 in *__GI___libc_realloc (oldmem=0xb7401fb8, bytes=128) at malloc.c:3425
#9 0x081cced6 in default_realloc_ex (str=0xb7401fb8, num=128, file=0x829df09 "lhash.c", line=342) at mem.c:86
#10 0x081cd4ff in CRYPTO_realloc (str=0xb7401fb8, num=128, file=0x829df09 "lhash.c", line=342) at mem.c:331
#11 0x081e851a in expand (lh=0xb7401f50) at lhash.c:341
#12 0x081e81a0 in lh_insert (lh=0xb7401f50, data="" at lhash.c:187
#13 0x081cfbd8 in OBJ_NAME_add (name=0x828ef27 "ssl2-md5", type=1, data="" "MD5") at o_names.c:203
#14 0x081cc5a4 in SSL_library_init () at ssl_algs.c:100

See attached patch file for fix.

This doesn't appear to be an exploitable security vulnerability as far as I can tell. This bug appears to have existed as far back as 1999 based on looking at http://cvs.openssl.org/rlog?f=openssl/crypto/objects/o_names.c

[openssl.org #1743] crasher due to lack of threadsafety on names_lh

Reply via email to