diff options
author | Marcus Huewe <suse-tux@gmx.de> | 2018-05-11 12:24:56 +0200 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2018-06-07 13:12:39 +0100 |
commit | 6849421c7553337df64536893690478f99f053c9 (patch) | |
tree | 2f89752f674561b2f1482f32bea52a8ad1a8dc72 | |
parent | 853d2453620ff04fbe8afa89fabd77eff42f54db (diff) |
Do not free a session before calling the remove_session_cb
If the remove_session_cb accesses the session's data (for instance,
via SSL_SESSION_get_protocol_version), a potential use after free
can occur. For this, consider the following scenario when adding
a new session via SSL_CTX_add_session:
- The session cache is full
(SSL_CTX_sess_number(ctx) > SSL_CTX_sess_get_cache_size(ctx))
- Only the session cache has a reference to ctx->session_cache_tail
(that is, ctx->session_cache_tail->references == 1)
Since the cache is full, remove_session_lock is called to remove
ctx->session_cache_tail from the cache. That is, it
SSL_SESSION_free()s the session, which free()s the data. Afterwards,
the free()d session is passed to the remove_session_cb. If the callback
accesses the session's data, we have a use after free.
The free before calling the callback behavior was introduced in
commit e4612d02c53cccd24fa97b08fc01250d1238cca1 ("Remove sessions
from external cache, even if internal cache not used.").
CLA: trivial
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6222)
(cherry picked from commit c0a58e034d3eff68ca5e0d36d7b4d147425b0599)
-rw-r--r-- | ssl/ssl_sess.c | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 0dea8b5224..023ba9d893 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -734,11 +734,11 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck) if (lck) CRYPTO_THREAD_unlock(ctx->lock); - if (ret) - SSL_SESSION_free(r); - if (ctx->remove_session_cb != NULL) ctx->remove_session_cb(ctx, c); + + if (ret) + SSL_SESSION_free(r); } else ret = 0; return (ret); |