Skip to content

Commit 28e9ed4

Browse files
colesburycolesbury
committed
pythongh-143756: Avoid borrowed reference in SSL code
GET_SOCKET() returned a borrowed reference, which was potentially unsafe. Also, refactor out some common code.
1 parent 2873c31 commit 28e9ed4

File tree

1 file changed

+46
-98
lines changed

1 file changed

+46
-98
lines changed

‎Modules/_ssl.c‎

Lines changed: 46 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -420,26 +420,6 @@ typedef enum {
420420
#define ERRSTR1(x,y,z) (x ":" y ": " z)
421421
#define ERRSTR(x) ERRSTR1("_ssl.c", Py_STRINGIFY(__LINE__), x)
422422

423-
// Get the socket from a PySSLSocket, if it has one.
424-
// Return a borrowed reference.
425-
static inline PySocketSockObject* GET_SOCKET(PySSLSocket *obj) {
426-
if (obj->Socket) {
427-
PyObject *sock;
428-
if (PyWeakref_GetRef(obj->Socket, &sock)) {
429-
// GET_SOCKET() returns a borrowed reference
430-
Py_DECREF(sock);
431-
}
432-
else {
433-
// dead weak reference
434-
sock = Py_None;
435-
}
436-
return (PySocketSockObject *)sock; // borrowed reference
437-
}
438-
else {
439-
return NULL;
440-
}
441-
}
442-
443423
/* If sock is NULL, use a timeout of 0 second */
444424
#define GET_SOCKET_TIMEOUT(sock) \
445425
((sock != NULL) ? (sock)->sock_timeout : 0)
@@ -791,6 +771,35 @@ _ssl_deprecated(const char* msg, int stacklevel) {
791771
#define PY_SSL_DEPRECATED(name, stacklevel, ret) \
792772
if (_ssl_deprecated((name), (stacklevel)) == -1) return (ret)
793773

774+
// Get the socket from a PySSLSocket, if it has one.
775+
// Stores a strong reference in out_sock.
776+
static int
777+
get_socket(PySSLSocket *obj, PySocketSockObject **out_sock,
778+
const char *filename, int lineno)
779+
{
780+
if (!obj->Socket) {
781+
*out_sock = NULL;
782+
return 0;
783+
}
784+
PySocketSockObject *sock;
785+
int res = PyWeakref_GetRef(obj->Socket, (PyObject **)&sock);
786+
if (res == 0 || sock->sock_fd == INVALID_SOCKET) {
787+
_setSSLError(get_state_sock(obj),
788+
"Underlying socket connection gone",
789+
PY_SSL_ERROR_NO_SOCKET, filename, lineno);
790+
*out_sock = NULL;
791+
return -1;
792+
}
793+
if (sock != NULL) {
794+
/* just in case the blocking state of the socket has been changed */
795+
int nonblocking = (sock->sock_timeout >= 0);
796+
BIO_set_nbio(SSL_get_rbio(obj->ssl), nonblocking);
797+
BIO_set_nbio(SSL_get_wbio(obj->ssl), nonblocking);
798+
}
799+
*out_sock = sock;
800+
return res;
801+
}
802+
794803
/*
795804
* SSL objects
796805
*/
@@ -1018,24 +1027,13 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10181027
int ret;
10191028
_PySSLError err;
10201029
PyObject *exc = NULL;
1021-
int sockstate, nonblocking;
1022-
PySocketSockObject *sock = GET_SOCKET(self);
1030+
int sockstate;
10231031
PyTime_t timeout, deadline = 0;
10241032
int has_timeout;
10251033

1026-
if (sock) {
1027-
if (((PyObject*)sock) == Py_None) {
1028-
_setSSLError(get_state_sock(self),
1029-
"Underlying socket connection gone",
1030-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
1031-
return NULL;
1032-
}
1033-
Py_INCREF(sock);
1034-
1035-
/* just in case the blocking state of the socket has been changed */
1036-
nonblocking = (sock->sock_timeout >= 0);
1037-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
1038-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
1034+
PySocketSockObject *sock = NULL;
1035+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
1036+
return NULL;
10391037
}
10401038

10411039
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2607,22 +2605,12 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset,
26072605
int sockstate;
26082606
_PySSLError err;
26092607
PyObject *exc = NULL;
2610-
PySocketSockObject *sock = GET_SOCKET(self);
26112608
PyTime_t timeout, deadline = 0;
26122609
int has_timeout;
26132610

2614-
if (sock != NULL) {
2615-
if ((PyObject *)sock == Py_None) {
2616-
_setSSLError(get_state_sock(self),
2617-
"Underlying socket connection gone",
2618-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
2619-
return NULL;
2620-
}
2621-
Py_INCREF(sock);
2622-
/* just in case the blocking state of the socket has been changed */
2623-
int nonblocking = (sock->sock_timeout >= 0);
2624-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
2625-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
2611+
PySocketSockObject *sock = NULL;
2612+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
2613+
return NULL;
26262614
}
26272615

26282616
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2744,26 +2732,12 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
27442732
int sockstate;
27452733
_PySSLError err;
27462734
PyObject *exc = NULL;
2747-
int nonblocking;
2748-
PySocketSockObject *sock = GET_SOCKET(self);
27492735
PyTime_t timeout, deadline = 0;
27502736
int has_timeout;
27512737

2752-
if (sock != NULL) {
2753-
if (((PyObject*)sock) == Py_None) {
2754-
_setSSLError(get_state_sock(self),
2755-
"Underlying socket connection gone",
2756-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
2757-
return NULL;
2758-
}
2759-
Py_INCREF(sock);
2760-
}
2761-
2762-
if (sock != NULL) {
2763-
/* just in case the blocking state of the socket has been changed */
2764-
nonblocking = (sock->sock_timeout >= 0);
2765-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
2766-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
2738+
PySocketSockObject *sock = NULL;
2739+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
2740+
return NULL;
27672741
}
27682742

27692743
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2893,8 +2867,6 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
28932867
int sockstate;
28942868
_PySSLError err;
28952869
PyObject *exc = NULL;
2896-
int nonblocking;
2897-
PySocketSockObject *sock = GET_SOCKET(self);
28982870
PyTime_t timeout, deadline = 0;
28992871
int has_timeout;
29002872

@@ -2903,14 +2875,9 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
29032875
return NULL;
29042876
}
29052877

2906-
if (sock != NULL) {
2907-
if (((PyObject*)sock) == Py_None) {
2908-
_setSSLError(get_state_sock(self),
2909-
"Underlying socket connection gone",
2910-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
2911-
return NULL;
2912-
}
2913-
Py_INCREF(sock);
2878+
PySocketSockObject *sock = NULL;
2879+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
2880+
return NULL;
29142881
}
29152882

29162883
if (!group_right_1) {
@@ -2941,13 +2908,6 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
29412908
}
29422909
}
29432910

2944-
if (sock != NULL) {
2945-
/* just in case the blocking state of the socket has been changed */
2946-
nonblocking = (sock->sock_timeout >= 0);
2947-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
2948-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
2949-
}
2950-
29512911
timeout = GET_SOCKET_TIMEOUT(sock);
29522912
has_timeout = (timeout > 0);
29532913
if (has_timeout)
@@ -3038,26 +2998,14 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
30382998
{
30392999
_PySSLError err;
30403000
PyObject *exc = NULL;
3041-
int sockstate, nonblocking, ret;
3001+
int sockstate, ret;
30423002
int zeros = 0;
3043-
PySocketSockObject *sock = GET_SOCKET(self);
30443003
PyTime_t timeout, deadline = 0;
30453004
int has_timeout;
30463005

3047-
if (sock != NULL) {
3048-
/* Guard against closed socket */
3049-
if ((((PyObject*)sock) == Py_None) || (sock->sock_fd == INVALID_SOCKET)) {
3050-
_setSSLError(get_state_sock(self),
3051-
"Underlying socket connection gone",
3052-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
3053-
return NULL;
3054-
}
3055-
Py_INCREF(sock);
3056-
3057-
/* Just in case the blocking state of the socket has been changed */
3058-
nonblocking = (sock->sock_timeout >= 0);
3059-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
3060-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
3006+
PySocketSockObject *sock = NULL;
3007+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
3008+
return NULL;
30613009
}
30623010

30633011
timeout = GET_SOCKET_TIMEOUT(sock);

0 commit comments

Comments
 (0)