Skip to content

Commit 63d7bab

Browse files
committed
Fix SNI callback callable race use-after-free
In Modules/_ssl.c
1 parent be11d8b commit 63d7bab

1 file changed

Lines changed: 15 additions & 8 deletions

File tree

Modules/_ssl.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
#define OPENSSL_NO_DEPRECATED 1
2727

2828
#include "Python.h"
29+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
2930
#include "pycore_fileutils.h" // _PyIsSelectable_fd()
31+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED()
3032
#include "pycore_long.h" // _PyLong_UnsignedLongLong_Converter()
3133
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
3234
#include "pycore_time.h" // _PyDeadline_Init()
@@ -5153,12 +5155,15 @@ _servername_callback(SSL *s, int *al, void *args)
51535155
PyObject *result;
51545156
/* The high-level ssl.SSLSocket object */
51555157
PyObject *ssl_socket;
5158+
PyObject *sni_cb;
51565159
const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
51575160
PyGILState_STATE gstate = PyGILState_Ensure();
51585161

5159-
if (sslctx->set_sni_cb == NULL) {
5160-
/* remove race condition in this the call back while if removing the
5161-
* callback is in progress */
5162+
Py_BEGIN_CRITICAL_SECTION(sslctx);
5163+
sni_cb = Py_XNewRef(FT_ATOMIC_LOAD_PTR_RELAXED(sslctx->set_sni_cb));
5164+
Py_END_CRITICAL_SECTION();
5165+
5166+
if (sni_cb == NULL) {
51625167
PyGILState_Release(gstate);
51635168
return SSL_TLSEXT_ERR_OK;
51645169
}
@@ -5185,7 +5190,7 @@ _servername_callback(SSL *s, int *al, void *args)
51855190
goto error;
51865191

51875192
if (servername == NULL) {
5188-
result = PyObject_CallFunctionObjArgs(sslctx->set_sni_cb, ssl_socket,
5193+
result = PyObject_CallFunctionObjArgs(sni_cb, ssl_socket,
51895194
Py_None, sslctx, NULL);
51905195
}
51915196
else {
@@ -5212,7 +5217,7 @@ _servername_callback(SSL *s, int *al, void *args)
52125217
}
52135218
Py_DECREF(servername_bytes);
52145219
result = PyObject_CallFunctionObjArgs(
5215-
sslctx->set_sni_cb, ssl_socket, servername_str,
5220+
sni_cb, ssl_socket, servername_str,
52165221
sslctx, NULL);
52175222
Py_DECREF(servername_str);
52185223
}
@@ -5222,7 +5227,7 @@ _servername_callback(SSL *s, int *al, void *args)
52225227
PyErr_FormatUnraisable("Exception ignored "
52235228
"in ssl servername callback "
52245229
"while calling set SNI callback %R",
5225-
sslctx->set_sni_cb);
5230+
sni_cb);
52265231
*al = SSL_AD_HANDSHAKE_FAILURE;
52275232
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
52285233
}
@@ -5247,11 +5252,13 @@ _servername_callback(SSL *s, int *al, void *args)
52475252
Py_DECREF(result);
52485253
}
52495254

5255+
Py_DECREF(sni_cb);
52505256
PyGILState_Release(gstate);
52515257
return ret;
52525258

52535259
error:
52545260
Py_XDECREF(ssl_socket);
5261+
Py_DECREF(sni_cb);
52555262
*al = SSL_AD_INTERNAL_ERROR;
52565263
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
52575264
PyGILState_Release(gstate);
@@ -5277,7 +5284,7 @@ static PyObject *
52775284
_ssl__SSLContext_sni_callback_get_impl(PySSLContext *self)
52785285
/*[clinic end generated code: output=961e6575cdfaf036 input=3aee06696b0874d9]*/
52795286
{
5280-
PyObject *cb = self->set_sni_cb;
5287+
PyObject *cb = FT_ATOMIC_LOAD_PTR_RELAXED(self->set_sni_cb);
52815288
if (cb == NULL) {
52825289
Py_RETURN_NONE;
52835290
}
@@ -5312,7 +5319,7 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value)
53125319
"not a callable object");
53135320
return -1;
53145321
}
5315-
self->set_sni_cb = Py_NewRef(value);
5322+
FT_ATOMIC_STORE_PTR_RELAXED(self->set_sni_cb, Py_NewRef(value));
53165323
SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback);
53175324
SSL_CTX_set_tlsext_servername_arg(self->ctx, self);
53185325
}

0 commit comments

Comments
 (0)