public/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Adds missing checks of return from XXX_up_ref().

Browse source 
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26294)
This commit is contained in:
Frederik Wedel-Heinen 2024-12-28 10:13:48 +01:00 • committed by Tomas Mraz
parent e9aac2c2f3
commit 00fbc96988
45 changed files with 447 additions and 241 deletions
Download patch file Download diff file Expand all files Collapse all files

11
ssl/bio_ssl.c
View file

@ -299,10 +299,13 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
bs->ssl = ssl;
bio = SSL_get_rbio(ssl);
if (bio != NULL) {
if (!BIO_up_ref(bio)) {
ret = 0;
break;
}
if (next != NULL)
BIO_push(bio, next);
BIO_set_next(b, bio);
BIO_up_ref(bio);
}
BIO_set_init(b, 1);
break;
@ -338,8 +341,10 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
* We are going to pass ownership of next to the SSL object...but
* we don't own a reference to pass yet - so up ref
*/
BIO_up_ref(next);
SSL_set_bio(ssl, next, next);
if (!BIO_up_ref(next))
ret = 0;
else
SSL_set_bio(ssl, next, next);
}
break;
case BIO_CTRL_POP:

8
ssl/s3_lib.c
View file

@ -3854,7 +3854,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
if (sc->session == NULL || sc->s3.peer_tmp == NULL) {
return 0;
} else {
EVP_PKEY_up_ref(sc->s3.peer_tmp);
if (!EVP_PKEY_up_ref(sc->s3.peer_tmp))
return 0;
*(EVP_PKEY **)parg = sc->s3.peer_tmp;
return 1;
}
@ -3863,7 +3865,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
if (sc->session == NULL || sc->s3.tmp.pkey == NULL) {
return 0;
} else {
EVP_PKEY_up_ref(sc->s3.tmp.pkey);
if (!EVP_PKEY_up_ref(sc->s3.tmp.pkey))
return 0;
*(EVP_PKEY **)parg = sc->s3.tmp.pkey;
return 1;
}

29
ssl/ssl_cert.c
View file

@ -119,8 +119,9 @@ CERT *ssl_cert_dup(CERT *cert)
}
if (cert->dh_tmp != NULL) {
if (!EVP_PKEY_up_ref(cert->dh_tmp))
goto err;
ret->dh_tmp = cert->dh_tmp;
EVP_PKEY_up_ref(ret->dh_tmp);
}
ret->dh_tmp_cb = cert->dh_tmp_cb;
@ -131,13 +132,15 @@ CERT *ssl_cert_dup(CERT *cert)
CERT_PKEY *rpk = ret->pkeys + i;
if (cpk->x509 != NULL) {
if (!X509_up_ref(cpk->x509))
goto err;
rpk->x509 = cpk->x509;
X509_up_ref(rpk->x509);
}
if (cpk->privatekey != NULL) {
if (!EVP_PKEY_up_ref(cpk->privatekey))
goto err;
rpk->privatekey = cpk->privatekey;
EVP_PKEY_up_ref(cpk->privatekey);
}
if (cpk->chain) {
@ -201,12 +204,14 @@ CERT *ssl_cert_dup(CERT *cert)
ret->cert_cb_arg = cert->cert_cb_arg;
if (cert->verify_store) {
X509_STORE_up_ref(cert->verify_store);
if (!X509_STORE_up_ref(cert->verify_store))
goto err;
ret->verify_store = cert->verify_store;
}
if (cert->chain_store) {
X509_STORE_up_ref(cert->chain_store);
if (!X509_STORE_up_ref(cert->chain_store))
goto err;
ret->chain_store = cert->chain_store;
}
@ -350,9 +355,12 @@ int ssl_cert_add0_chain_cert(SSL_CONNECTION *s, SSL_CTX *ctx, X509 *x)
int ssl_cert_add1_chain_cert(SSL_CONNECTION *s, SSL_CTX *ctx, X509 *x)
{
if (!ssl_cert_add0_chain_cert(s, ctx, x))
if (!X509_up_ref(x))
return 0;
X509_up_ref(x);
if (!ssl_cert_add0_chain_cert(s, ctx, x)) {
X509_free(x);
return 0;
}
return 1;
}
@ -1162,14 +1170,17 @@ int ssl_build_cert_chain(SSL_CONNECTION *s, SSL_CTX *ctx, int flags)
int ssl_cert_set_cert_store(CERT *c, X509_STORE *store, int chain, int ref)
{
X509_STORE **pstore;
if (ref && store && !X509_STORE_up_ref(store))
return 0;
if (chain)
pstore = &c->chain_store;
else
pstore = &c->verify_store;
X509_STORE_free(*pstore);
*pstore = store;
if (ref && store)
X509_STORE_up_ref(store);
return 1;
}

67
ssl/ssl_lib.c
View file

@ -703,30 +703,30 @@ SSL *SSL_new(SSL_CTX *ctx)
int ossl_ssl_init(SSL *ssl, SSL_CTX *ctx, const SSL_METHOD *method, int type)
{
ssl->type = type;
if (!SSL_CTX_up_ref(ctx))
return 0;
ssl->lock = CRYPTO_THREAD_lock_new();
if (ssl->lock == NULL)
return 0;
if (!CRYPTO_NEW_REF(&ssl->references, 1)) {
CRYPTO_THREAD_lock_free(ssl->lock);
return 0;
}
if (ssl->lock == NULL || !CRYPTO_NEW_REF(&ssl->references, 1))
goto err;
if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, ssl, &ssl->ex_data)) {
CRYPTO_THREAD_lock_free(ssl->lock);
CRYPTO_FREE_REF(&ssl->references);
ssl->lock = NULL;
return 0;
goto err;
}
SSL_CTX_up_ref(ctx);
ssl->ctx = ctx;
ssl->type = type;
ssl->defltmeth = ssl->method = method;
return 1;
err:
CRYPTO_THREAD_lock_free(ssl->lock);
ssl->lock = NULL;
SSL_CTX_free(ctx);
return 0;
}
SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, SSL *user_ssl,
@ -824,7 +824,10 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, SSL *user_ssl,
s->ext.ocsp.exts = NULL;
s->ext.ocsp.resp = NULL;
s->ext.ocsp.resp_len = 0;
SSL_CTX_up_ref(ctx);
if (!SSL_CTX_up_ref(ctx))
goto err;
s->session_ctx = ctx;
if (ctx->ext.ecpointformats) {
s->ext.ecpointformats =
@ -1964,8 +1967,8 @@ X509 *SSL_get1_peer_certificate(const SSL *s)
{
X509 *r = SSL_get0_peer_certificate(s);
if (r != NULL)
X509_up_ref(r);
if (r != NULL && !X509_up_ref(r))
return NULL;
return r;
}
@ -4726,8 +4729,7 @@ void ssl_update_cache(SSL_CONNECTION *s, int mode)
* TLSv1.3 without early data because some applications just want to
* know about the creation of a session and aren't doing a full cache.
*/
if (s->session_ctx->new_session_cb != NULL) {
SSL_SESSION_up_ref(s->session);
if (s->session_ctx->new_session_cb != NULL && SSL_SESSION_up_ref(s->session)) {
if (!s->session_ctx->new_session_cb(SSL_CONNECTION_GET_USER_SSL(s),
s->session))
SSL_SESSION_free(s->session);
@ -5437,24 +5439,19 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx)
if (ctx == NULL)
ctx = sc->session_ctx;
new_cert = ssl_cert_dup(ctx->cert);
if (new_cert == NULL) {
return NULL;
}
if (!custom_exts_copy_flags(&new_cert->custext, &sc->cert->custext)) {
ssl_cert_free(new_cert);
return NULL;
}
ssl_cert_free(sc->cert);
sc->cert = new_cert;
if (new_cert == NULL)
goto err;
if (!custom_exts_copy_flags(&new_cert->custext, &sc->cert->custext))
goto err;
/*
* Program invariant: |sid_ctx| has fixed size (SSL_MAX_SID_CTX_LENGTH),
* so setter APIs must prevent invalid lengths from entering the system.
*/
if (!ossl_assert(sc->sid_ctx_length <= sizeof(sc->sid_ctx)))
return NULL;
goto err;
if (!SSL_CTX_up_ref(ctx))
goto err;
/*
* If the session ID context matches that of the parent SSL_CTX,
@ -5469,11 +5466,16 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx)
memcpy(&sc->sid_ctx, &ctx->sid_ctx, sizeof(sc->sid_ctx));
}
SSL_CTX_up_ref(ctx);
ssl_cert_free(sc->cert);
sc->cert = new_cert;
SSL_CTX_free(ssl->ctx); /* decrement reference count */
ssl->ctx = ctx;
return ssl->ctx;
err:
ssl_cert_free(new_cert);
return NULL;
}
int SSL_CTX_set_default_verify_paths(SSL_CTX *ctx)
@ -5698,8 +5700,9 @@ void SSL_CTX_set_cert_store(SSL_CTX *ctx, X509_STORE *store)
void SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store)
{
if (store != NULL)
X509_STORE_up_ref(store);
if (store != NULL && !X509_STORE_up_ref(store))
return;
SSL_CTX_set_cert_store(ctx, store);
}

19
ssl/ssl_rsa.c
View file

@ -142,9 +142,10 @@ static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey, SSL_CTX *ctx)
if (c->pkeys[i].x509 != NULL
&& !X509_check_private_key(c->pkeys[i].x509, pkey))
return 0;
if (!EVP_PKEY_up_ref(pkey))
return 0;
EVP_PKEY_free(c->pkeys[i].privatekey);
EVP_PKEY_up_ref(pkey);
c->pkeys[i].privatekey = pkey;
c->key = &c->pkeys[i];
return 1;
@ -296,8 +297,10 @@ static int ssl_set_cert(CERT *c, X509 *x, SSL_CTX *ctx)
}
}
if (!X509_up_ref(x))
return 0;
X509_free(c->pkeys[i].x509);
X509_up_ref(x);
c->pkeys[i].x509 = x;
c->key = &(c->pkeys[i]);
@ -1047,21 +1050,27 @@ static int ssl_set_cert_and_key(SSL *ssl, SSL_CTX *ctx, X509 *x509, EVP_PKEY *pr
if (chain != NULL) {
dup_chain = X509_chain_up_ref(chain);
if (dup_chain == NULL) {
if (dup_chain == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_X509_LIB);
goto out;
}
}
if (!X509_up_ref(x509))
goto out;
if (!EVP_PKEY_up_ref(privatekey)) {
X509_free(x509);
goto out;
}
OSSL_STACK_OF_X509_free(c->pkeys[i].chain);
c->pkeys[i].chain = dup_chain;
X509_free(c->pkeys[i].x509);
X509_up_ref(x509);
c->pkeys[i].x509 = x509;
EVP_PKEY_free(c->pkeys[i].privatekey);
EVP_PKEY_up_ref(privatekey);
c->pkeys[i].privatekey = privatekey;
c->key = &(c->pkeys[i]);

12
ssl/ssl_rsa_legacy.c
View file

@ -28,7 +28,11 @@ int SSL_use_RSAPrivateKey(SSL *ssl, RSA *rsa)
return 0;
}
RSA_up_ref(rsa);
if (!RSA_up_ref(rsa)) {
EVP_PKEY_free(pkey);
return 0;
}
if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) {
RSA_free(rsa);
EVP_PKEY_free(pkey);
@ -115,7 +119,11 @@ int SSL_CTX_use_RSAPrivateKey(SSL_CTX *ctx, RSA *rsa)
return 0;
}
RSA_up_ref(rsa);
if (!RSA_up_ref(rsa)) {
EVP_PKEY_free(pkey);
return 0;
}
if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) {
RSA_free(rsa);
EVP_PKEY_free(pkey);

28
ssl/ssl_sess.c
View file

@ -83,8 +83,8 @@ SSL_SESSION *SSL_get1_session(SSL *ssl)
if (!CRYPTO_THREAD_read_lock(ssl->lock))
return NULL;
sess = SSL_get_session(ssl);
if (sess != NULL)
SSL_SESSION_up_ref(sess);
if (sess != NULL && !SSL_SESSION_up_ref(sess))
sess = NULL;
CRYPTO_THREAD_unlock(ssl->lock);
return sess;
}
@ -513,7 +513,10 @@ SSL_SESSION *lookup_sess_in_cache(SSL_CONNECTION *s,
ret = lh_SSL_SESSION_retrieve(s->session_ctx->sessions, &data);
if (ret != NULL) {
/* don't allow other threads to steal it: */
SSL_SESSION_up_ref(ret);
if (!SSL_SESSION_up_ref(ret)) {
CRYPTO_THREAD_unlock(s->session_ctx->lock);
return NULL;
}
}
CRYPTO_THREAD_unlock(s->session_ctx->lock);
if (ret == NULL)
@ -543,8 +546,8 @@ SSL_SESSION *lookup_sess_in_cache(SSL_CONNECTION *s,
* reference count itself [i.e. copy == 0], or things won't be
* thread-safe).
*/
if (copy)
SSL_SESSION_up_ref(ret);
if (copy && !SSL_SESSION_up_ref(ret))
return NULL;
/*
* Add the externally cached session to the internal cache as
@ -727,7 +730,8 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
* it has two ways of access: each session is in a doubly linked list and
* an lhash
*/
SSL_SESSION_up_ref(c);
if (!SSL_SESSION_up_ref(c))
return 0;
/*
* if session c is in already in cache, we take back the increment later
*/
@ -891,16 +895,20 @@ int SSL_set_session(SSL *s, SSL_SESSION *session)
if (sc == NULL)
return 0;
if (session != NULL && !SSL_SESSION_up_ref(session))
return 0;
ssl_clear_bad_session(sc);
if (s->defltmeth != s->method) {
if (!SSL_set_ssl_method(s, s->defltmeth))
if (!SSL_set_ssl_method(s, s->defltmeth)) {
SSL_SESSION_free(session);
return 0;
}
}
if (session != NULL) {
SSL_SESSION_up_ref(session);
if (session != NULL)
sc->verify_result = session->verify_result;
}
SSL_SESSION_free(sc->session);
sc->session = session;

6
ssl/statem/statem_clnt.c
View file

@ -2141,8 +2141,12 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s,
}
}
if (!X509_up_ref(x)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return WORK_ERROR;
}
X509_free(s->session->peer);
X509_up_ref(x);
s->session->peer = x;
s->session->verify_result = s->verify_result;
/* Ensure there is no RPK */