Fix potential use-after-free in REF_PRINT_COUNT

We use REF_PRINT_COUNT to dump out the value of various reference
counters in our code

However, we commonly use this macro after an increment or decrement.  On
increment its fine, but on decrement its not, because the macro
dereferences the object holding the counter value, which may be freed by
another thread, as we've given up our ref count to it prior to using the
macro.

The rule is that we can't reference memory for an object once we've
released our reference, so lets fix this by altering REF_PRINT_COUNT to
accept the value returned by CRYPTO_[UP|DOWN]_REF instead.  The
eliminates the need to dereference the memory the object points to an
allows us to use the call after we release our reference count

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25664)

(cherry picked from commit dc10ffc2834e0d2f5ebc1c3e29bd97f1f43a0ead)

diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index b3605b844ef324e64edc1ae61eb5e4ab309507e6..a9ece88ad37a869cda6982f996defc1f8a57cfd3 100644 (file)
--- a/crypto/bio/bio_lib.c
+++ b/crypto/bio/bio_lib.c
@@ -126,7 +126,7 @@ int BIO_free(BIO *a)
     if (CRYPTO_DOWN_REF(&a->references, &ret) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("BIO", a);
+    REF_PRINT_COUNT("BIO", ret, a);
     if (ret > 0)
         return 1;
     REF_ASSERT_ISNT(ret < 0);
@@ -191,7 +191,7 @@ int BIO_up_ref(BIO *a)
     if (CRYPTO_UP_REF(&a->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("BIO", a);
+    REF_PRINT_COUNT("BIO", i, a);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c
index 9d5a6b0b6c2baa7951fdf770ce70057cdfba023b..93e08b3f8c71bd0a573fefd08e4e15d857222a73 100644 (file)
--- a/crypto/dh/dh_lib.c
+++ b/crypto/dh/dh_lib.c
@@ -141,7 +141,7 @@ void DH_free(DH *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("DH", r);
+    REF_PRINT_COUNT("DH", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -171,7 +171,7 @@ int DH_up_ref(DH *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DH", r);
+    REF_PRINT_COUNT("DH", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/dsa/dsa_lib.c b/crypto/dsa/dsa_lib.c
index 7997c2ac25e459d04acf71828864ed3ac3a5f939..db6e3b059b4deebc68f9bff66c071d87ac04f865 100644 (file)
--- a/crypto/dsa/dsa_lib.c
+++ b/crypto/dsa/dsa_lib.c
@@ -218,7 +218,7 @@ void DSA_free(DSA *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("DSA", r);
+    REF_PRINT_COUNT("DSA", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -249,7 +249,7 @@ int DSA_up_ref(DSA *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSA", r);
+    REF_PRINT_COUNT("DSA", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/dso/dso_lib.c b/crypto/dso/dso_lib.c
index 8f3387e9b802085420ac49c6c65615226215eda1..65579cb8b360780c8d6d532913cbe65d9ab85541 100644 (file)
--- a/crypto/dso/dso_lib.c
+++ b/crypto/dso/dso_lib.c
@@ -54,7 +54,7 @@ int DSO_free(DSO *dso)
     if (CRYPTO_DOWN_REF(&dso->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSO", dso);
+    REF_PRINT_COUNT("DSO", i, dso);
     if (i > 0)
         return 1;
     REF_ASSERT_ISNT(i < 0);
@@ -96,7 +96,7 @@ int DSO_up_ref(DSO *dso)
     if (CRYPTO_UP_REF(&dso->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("DSO", dso);
+    REF_PRINT_COUNT("DSO", i, dso);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c
index 9bc4e032c557171925e732e01217b59af060b31b..823b5c8c8e6c6c13d528b964403a89e36845ff28 100644 (file)
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -76,7 +76,7 @@ void EC_KEY_free(EC_KEY *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("EC_KEY", r);
+    REF_PRINT_COUNT("EC_KEY", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -197,7 +197,7 @@ int EC_KEY_up_ref(EC_KEY *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("EC_KEY", r);
+    REF_PRINT_COUNT("EC_KEY", i, r);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index 9eb007cdf908285b4a26ae0eee1f8a2137a8c394..e9092a6c9db433dd81bf0880fbf935a8bb52efff 100644 (file)
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -85,7 +85,7 @@ void EC_ec_pre_comp_free(EC_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_ec", pre);
+    REF_PRINT_COUNT("EC_ec", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c
index debfdb3dc942e394d933b0b9f48817963f75fcb3..6485f46f717c4d566f4b5a8ce98b494aa7116179 100644 (file)
--- a/crypto/ec/ecp_nistp224.c
+++ b/crypto/ec/ecp_nistp224.c
@@ -1264,7 +1264,7 @@ void EC_nistp224_pre_comp_free(NISTP224_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("EC_nistp224", p);
+    REF_PRINT_COUNT("EC_nistp224", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp256.c b/crypto/ec/ecp_nistp256.c
index d28306a6bd15be35d3950811d0bfb74512141b74..2cf4212685ded2a6d11b4e8947269162374f4370 100644 (file)
--- a/crypto/ec/ecp_nistp256.c
+++ b/crypto/ec/ecp_nistp256.c
@@ -1874,7 +1874,7 @@ void EC_nistp256_pre_comp_free(NISTP256_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_nistp256", pre);
+    REF_PRINT_COUNT("EC_nistp256", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp384.c b/crypto/ec/ecp_nistp384.c
index ff68f9cc7ad023ec33a2ad7be02099a7ae748d06..3fd7a40020d64e3fe0716dd94fdf758b494e48e0 100644 (file)
--- a/crypto/ec/ecp_nistp384.c
+++ b/crypto/ec/ecp_nistp384.c
@@ -1560,7 +1560,7 @@ void ossl_ec_nistp384_pre_comp_free(NISTP384_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("ossl_ec_nistp384", p);
+    REF_PRINT_COUNT("ossl_ec_nistp384", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistp521.c b/crypto/ec/ecp_nistp521.c
index db5a9dd5def3691269d9fe769f0755fd1e4f6044..fe6836a147140c5dc00faa6ebf1817b7635c8be1 100644 (file)
--- a/crypto/ec/ecp_nistp521.c
+++ b/crypto/ec/ecp_nistp521.c
@@ -1766,7 +1766,7 @@ void EC_nistp521_pre_comp_free(NISTP521_PRE_COMP *p)
         return;
 
     CRYPTO_DOWN_REF(&p->references, &i);
-    REF_PRINT_COUNT("EC_nistp521", p);
+    REF_PRINT_COUNT("EC_nistp521", i, p);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecp_nistz256.c b/crypto/ec/ecp_nistz256.c
index 5760639a2ee2405b2413b672db347e898a426d4d..827c5c43584aa485a4298de65e1b6ded4cf05d7f 100644 (file)
--- a/crypto/ec/ecp_nistz256.c
+++ b/crypto/ec/ecp_nistz256.c
@@ -1238,7 +1238,7 @@ void EC_nistz256_pre_comp_free(NISTZ256_PRE_COMP *pre)
         return;
 
     CRYPTO_DOWN_REF(&pre->references, &i);
-    REF_PRINT_COUNT("EC_nistz256", pre);
+    REF_PRINT_COUNT("EC_nistz256", i, pre);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/ec/ecx_key.c b/crypto/ec/ecx_key.c
index ba725eb573c28cabf0199f889586e419000087c1..aeaf5783d10c77907704b6c94a646e964a693857 100644 (file)
--- a/crypto/ec/ecx_key.c
+++ b/crypto/ec/ecx_key.c
@@ -69,7 +69,7 @@ void ossl_ecx_key_free(ECX_KEY *key)
         return;
 
     CRYPTO_DOWN_REF(&key->references, &i);
-    REF_PRINT_COUNT("ECX_KEY", key);
+    REF_PRINT_COUNT("ECX_KEY", i, key);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -92,7 +92,7 @@ int ossl_ecx_key_up_ref(ECX_KEY *key)
     if (CRYPTO_UP_REF(&key->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("ECX_KEY", key);
+    REF_PRINT_COUNT("ECX_KEY", i, key);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index b7377751bd483518d83c3580004396ce2e6a4c74..aaba87170586e317447d52c5fa183ccc647a9fce 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1672,7 +1672,7 @@ int EVP_PKEY_up_ref(EVP_PKEY *pkey)
     if (CRYPTO_UP_REF(&pkey->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("EVP_PKEY", pkey);
+    REF_PRINT_COUNT("EVP_PKEY", i, pkey);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -1793,7 +1793,7 @@ void EVP_PKEY_free(EVP_PKEY *x)
         return;
 
     CRYPTO_DOWN_REF(&x->references, &i);
-    REF_PRINT_COUNT("EVP_PKEY", x);
+    REF_PRINT_COUNT("EVP_PKEY", i, x);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c
index 1c3b33c28b332b5ed60d1084fb32a891237c922b..608db90a9b8e335ef34812781c6c0d3a637b7a9d 100644 (file)
--- a/crypto/rsa/rsa_lib.c
+++ b/crypto/rsa/rsa_lib.c
@@ -141,7 +141,7 @@ void RSA_free(RSA *r)
         return;
 
     CRYPTO_DOWN_REF(&r->references, &i);
-    REF_PRINT_COUNT("RSA", r);
+    REF_PRINT_COUNT("RSA", i, r);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -188,7 +188,7 @@ int RSA_up_ref(RSA *r)
     if (CRYPTO_UP_REF(&r->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("RSA", r);
+    REF_PRINT_COUNT("RSA", i, r);
     REF_ASSERT_ISNT(i < 2);
     return i > 1 ? 1 : 0;
 }

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 0ca7cb960d4fd9f882c8fbda448f96822d74ed3f..8f6b884afcc3a2e2be15bc92eb1f217c4e83fe08 100644 (file)
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -232,7 +232,7 @@ void X509_STORE_free(X509_STORE *xs)
     if (xs == NULL)
         return;
     CRYPTO_DOWN_REF(&xs->references, &i);
-    REF_PRINT_COUNT("X509_STORE", xs);
+    REF_PRINT_COUNT("X509_STORE", i, xs);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -260,7 +260,7 @@ int X509_STORE_up_ref(X509_STORE *xs)
     if (CRYPTO_UP_REF(&xs->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509_STORE", xs);
+    REF_PRINT_COUNT("X509_STORE", i, xs);
     REF_ASSERT_ISNT(i < 2);
     return i > 1 ? 1 : 0;
 }

diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index 0881be7292b06b6605dbf8df252dfb68fdf63b0b..c05d44097125b58917b46282f850ddf046319814 100644 (file)
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c
@@ -119,7 +119,7 @@ int X509_up_ref(X509 *x)
     if (CRYPTO_UP_REF(&x->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509", x);
+    REF_PRINT_COUNT("X509", i, x);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 205fe3d6e5a4d01a72f40520dc7b55a7ded04675..e5dd4d5c3a3dd0cd1872f995bfc67cd2251c8494 100644 (file)
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c
@@ -78,7 +78,7 @@ int X509_CRL_up_ref(X509_CRL *crl)
     if (CRYPTO_UP_REF(&crl->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("X509_CRL", crl);
+    REF_PRINT_COUNT("X509_CRL", i, crl);
     REF_ASSERT_ISNT(i < 2);
     return i > 1;
 }

diff --git a/include/internal/refcount.h b/include/internal/refcount.h
index 0ccafc7753780afb336ec4733545ae285c5dc0ae..3740a034e146b09e23c830458949ffb669edfb5d 100644 (file)
--- a/include/internal/refcount.h
+++ b/include/internal/refcount.h
@@ -297,7 +297,7 @@ static ossl_unused ossl_inline void CRYPTO_FREE_REF(CRYPTO_REF_COUNT *refcnt)
 
 # define REF_PRINT_EX(text, count, object) \
     OSSL_TRACE3(REF_COUNT, "%p:%4d:%s\n", (object), (count), (text));
-# define REF_PRINT_COUNT(text, object) \
-    REF_PRINT_EX(text, object->references.val, (void *)object)
+# define REF_PRINT_COUNT(text, val, object) \
+    REF_PRINT_EX(text, val, (void *)object)
 
 #endif

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 96ec001eae4c5f92ca6ae3256a7ad26335f136c2..b242bd7478e7eb07971dd8cd2d3e55dd324549ef 100644 (file)
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -267,7 +267,7 @@ void ssl_cert_free(CERT *c)
     if (c == NULL)
         return;
     CRYPTO_DOWN_REF(&c->references, &i);
-    REF_PRINT_COUNT("CERT", c);
+    REF_PRINT_COUNT("CERT", i, c);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/ssl/ssl_cert_comp.c b/ssl/ssl_cert_comp.c
index 639610a5f77e2c9afadfa86f6bf174b790d4a5f0..4c7cdf26826e38b5811a1d568e81b03779085d50 100644 (file)
--- a/ssl/ssl_cert_comp.c
+++ b/ssl/ssl_cert_comp.c
@@ -136,7 +136,7 @@ void OSSL_COMP_CERT_free(OSSL_COMP_CERT *cc)
         return;
 
     CRYPTO_DOWN_REF(&cc->references, &i);
-    REF_PRINT_COUNT("OSSL_COMP_CERT", cc);
+    REF_PRINT_COUNT("OSSL_COMP_CERT", i, cc);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -152,7 +152,7 @@ int OSSL_COMP_CERT_up_ref(OSSL_COMP_CERT *cc)
     if (CRYPTO_UP_REF(&cc->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("OSSL_COMP_CERT", cc);
+    REF_PRINT_COUNT("OSSL_COMP_CERT", i, cc);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 87e8a1e3222c7b4f3dfd441982192f5daef57166..a6695ca4a03476efd50fe457c8a6f5c8450a68fa 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -976,7 +976,7 @@ int SSL_up_ref(SSL *s)
     if (CRYPTO_UP_REF(&s->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL", s);
+    REF_PRINT_COUNT("SSL", i, s);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -1379,7 +1379,7 @@ void SSL_free(SSL *s)
     if (s == NULL)
         return;
     CRYPTO_DOWN_REF(&s->references, &i);
-    REF_PRINT_COUNT("SSL", s);
+    REF_PRINT_COUNT("SSL", i, s);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -4121,7 +4121,7 @@ int SSL_CTX_up_ref(SSL_CTX *ctx)
     if (CRYPTO_UP_REF(&ctx->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL_CTX", ctx);
+    REF_PRINT_COUNT("SSL_CTX", i, ctx);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }
@@ -4135,7 +4135,7 @@ void SSL_CTX_free(SSL_CTX *a)
         return;
 
     CRYPTO_DOWN_REF(&a->references, &i);
-    REF_PRINT_COUNT("SSL_CTX", a);
+    REF_PRINT_COUNT("SSL_CTX", i, a);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);

diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 3639c4efe5c4a32d44362428aa923ab0e28b2e05..1134b234918663bc63928eaa0dc09780e9d12f57 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -843,7 +843,7 @@ void SSL_SESSION_free(SSL_SESSION *ss)
     if (ss == NULL)
         return;
     CRYPTO_DOWN_REF(&ss->references, &i);
-    REF_PRINT_COUNT("SSL_SESSION", ss);
+    REF_PRINT_COUNT("SSL_SESSION", i, ss);
     if (i > 0)
         return;
     REF_ASSERT_ISNT(i < 0);
@@ -877,7 +877,7 @@ int SSL_SESSION_up_ref(SSL_SESSION *ss)
     if (CRYPTO_UP_REF(&ss->references, &i) <= 0)
         return 0;
 
-    REF_PRINT_COUNT("SSL_SESSION", ss);
+    REF_PRINT_COUNT("SSL_SESSION", i, ss);
     REF_ASSERT_ISNT(i < 2);
     return ((i > 1) ? 1 : 0);
 }