Skip to content

Commit ee4e14a

Browse files
colesburycolesbury
authored
gh-143756: Avoid borrowed reference in SSL code (gh-143816)
GET_SOCKET() returned a borrowed reference, which was potentially unsafe. Also, refactor out some common code.
1 parent bcf9cb0 commit ee4e14a

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
@@ -423,26 +423,6 @@ typedef enum {
423423
#define ERRSTR1(x,y,z) (x ":" y ": " z)
424424
#define ERRSTR(x) ERRSTR1("_ssl.c", Py_STRINGIFY(__LINE__), x)
425425

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

777+
// Get the socket from a PySSLSocket, if it has one.
778+
// Stores a strong reference in out_sock.
779+
static int
780+
get_socket(PySSLSocket *obj, PySocketSockObject **out_sock,
781+
const char *filename, int lineno)
782+
{
783+
if (!obj->Socket) {
784+
*out_sock = NULL;
785+
return 0;
786+
}
787+
PySocketSockObject *sock;
788+
int res = PyWeakref_GetRef(obj->Socket, (PyObject **)&sock);
789+
if (res == 0 || sock->sock_fd == INVALID_SOCKET) {
790+
_setSSLError(get_state_sock(obj),
791+
"Underlying socket connection gone",
792+
PY_SSL_ERROR_NO_SOCKET, filename, lineno);
793+
*out_sock = NULL;
794+
return -1;
795+
}
796+
if (sock != NULL) {
797+
/* just in case the blocking state of the socket has been changed */
798+
int nonblocking = (sock->sock_timeout >= 0);
799+
BIO_set_nbio(SSL_get_rbio(obj->ssl), nonblocking);
800+
BIO_set_nbio(SSL_get_wbio(obj->ssl), nonblocking);
801+
}
802+
*out_sock = sock;
803+
return res;
804+
}
805+
797806
/*
798807
* SSL objects
799808
*/
@@ -1021,24 +1030,13 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10211030
int ret;
10221031
_PySSLError err;
10231032
PyObject *exc = NULL;
1024-
int sockstate, nonblocking;
1025-
PySocketSockObject *sock = GET_SOCKET(self);
1033+
int sockstate;
10261034
PyTime_t timeout, deadline = 0;
10271035
int has_timeout;
10281036

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

10441042
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2610,22 +2608,12 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset,
26102608
int sockstate;
26112609
_PySSLError err;
26122610
PyObject *exc = NULL;
2613-
PySocketSockObject *sock = GET_SOCKET(self);
26142611
PyTime_t timeout, deadline = 0;
26152612
int has_timeout;
26162613

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

26312619
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2747,26 +2735,12 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
27472735
int sockstate;
27482736
_PySSLError err;
27492737
PyObject *exc = NULL;
2750-
int nonblocking;
2751-
PySocketSockObject *sock = GET_SOCKET(self);
27522738
PyTime_t timeout, deadline = 0;
27532739
int has_timeout;
27542740

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

27722746
timeout = GET_SOCKET_TIMEOUT(sock);
@@ -2896,8 +2870,6 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
28962870
int sockstate;
28972871
_PySSLError err;
28982872
PyObject *exc = NULL;
2899-
int nonblocking;
2900-
PySocketSockObject *sock = GET_SOCKET(self);
29012873
PyTime_t timeout, deadline = 0;
29022874
int has_timeout;
29032875

@@ -2906,14 +2878,9 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
29062878
return NULL;
29072879
}
29082880

2909-
if (sock != NULL) {
2910-
if (((PyObject*)sock) == Py_None) {
2911-
_setSSLError(get_state_sock(self),
2912-
"Underlying socket connection gone",
2913-
PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
2914-
return NULL;
2915-
}
2916-
Py_INCREF(sock);
2881+
PySocketSockObject *sock = NULL;
2882+
if (get_socket(self, &sock, __FILE__, __LINE__) < 0) {
2883+
return NULL;
29172884
}
29182885

29192886
if (!group_right_1) {
@@ -2944,13 +2911,6 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
29442911
}
29452912
}
29462913

2947-
if (sock != NULL) {
2948-
/* just in case the blocking state of the socket has been changed */
2949-
nonblocking = (sock->sock_timeout >= 0);
2950-
BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
2951-
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
2952-
}
2953-
29542914
timeout = GET_SOCKET_TIMEOUT(sock);
29552915
has_timeout = (timeout > 0);
29562916
if (has_timeout)
@@ -3041,26 +3001,14 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
30413001
{
30423002
_PySSLError err;
30433003
PyObject *exc = NULL;
3044-
int sockstate, nonblocking, ret;
3004+
int sockstate, ret;
30453005
int zeros = 0;
3046-
PySocketSockObject *sock = GET_SOCKET(self);
30473006
PyTime_t timeout, deadline = 0;
30483007
int has_timeout;
30493008

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

30663014
timeout = GET_SOCKET_TIMEOUT(sock);

0 commit comments

Comments
 (0)