Originally found in 1.0.0j. Did not check versions between 1.0.0j and
1.0.1c since it is still present in the current version.
I am also worried about lines 139-143:
139:if (key-pkey != NULL)
140:{
141:CRYPTO_add(key-pkey-references, 1,
CRYPTO_LOCK_EVP_PKEY);
142:return key-pkey;
143:}
Assume thread 1 runs up to line 140 and some other thread does
EVP_PKEY_free() on the key before thread 1 can get the lock starting in
line 141. Am I overseeing some protection ?
Also note the same problem in lines 175-184:
175:CRYPTO_w_lock(CRYPTO_LOCK_EVP_PKEY);
176:if (key-pkey)
177:{
178:EVP_KEY_free(ret);
179:ret = key-pkey;
180:}
181:else
182:key-pkey = ret;
183:CRYPTO_w_unlock(CRYPTO_LOCK_EVP_PKEY);
184:CRYPTO_add(ret-references, 1, CRYPTO_LOCK_EVP_PKEY);
Assume thread 1 goes up until and finishes line 183, then thread 2 does
EVP_PKEY_free() on key.
I would have supplied patch suggestions for those places as well but I
am not aware of an existing openssl technique of manipulating the
reference counter *without* aquiring a lock, in case we already have a
lock of the correct type.
Regards,
Thomas
diff --git a/crypto/asn1/x_pubkey.c b/crypto/asn1/x_pubkey.c
index 627ec87..d8422ca 100644
--- a/crypto/asn1/x_pubkey.c
+++ b/crypto/asn1/x_pubkey.c
@@ -133,6 +133,7 @@ error:
EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key)
{
EVP_PKEY *ret=NULL;
+ EVP_PKEY *pktmp=NULL;
if (key == NULL) goto error;
@@ -175,7 +176,7 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key)
CRYPTO_w_lock(CRYPTO_LOCK_EVP_PKEY);
if (key-pkey)
{
- EVP_PKEY_free(ret);
+ pktmp=ret;
ret = key-pkey;
}
else
@@ -183,6 +184,8 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key)
CRYPTO_w_unlock(CRYPTO_LOCK_EVP_PKEY);
CRYPTO_add(ret-references, 1, CRYPTO_LOCK_EVP_PKEY);
+ if (pktmp)
+ EVP_KEY_free(pktmp);
return ret;
error:
