Issue 27773: Excessive Py_XDECREF in the ssl module:

Thomas E. Hybel reports:

This vulnerability exists in the function newPySSLSocket in /Modules/_ssl.c. The
problem is that Py_XDECREF is called on an object, self->server_hostname, which
isn't owned anymore.

The code looks like this:

    static PySSLSocket *
    newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
                   enum py_ssl_server_or_client socket_type,
                   char *server_hostname,
                   PySSLMemoryBIO *inbio, PySSLMemoryBIO *outbio)
    {
        PySSLSocket *self;
        ...
        if (server_hostname != NULL) {
            hostname = PyUnicode_Decode(server_hostname, strlen(server_hostname),
                                       "idna", "strict");
            ...
            self->server_hostname = hostname;
        }
        ...
        if (sock != NULL) {
            self->Socket = PyWeakref_NewRef((PyObject *) sock, NULL);
            if (self->Socket == NULL) {
                Py_DECREF(self);
                Py_XDECREF(self->server_hostname);
                return NULL;
            }
        }
    }

We're initializing the "self" variable. If a hostname was given as an argument,
we call PyUnicode_Decode to initialize self->server_hostname = hostname. At this
point both "self" and "self->server_hostname" have a reference count of 1.

Later on we set self->Socket to be a new weakref. However if the call to
PyWeakref_NewRef fails (the object cannot be weakly referenced) then we run
Py_DECREF(self). Since the reference count of "self" drops to 0, PySSL_dealloc
is called, which runs this line:

    Py_XDECREF(self->server_hostname);

Now self->server_hostname's refcount drops to 0 and it is freed.

Then, back in newPySSLSocket, we run Py_XDECREF(self->server_hostname); which is
inappropriate both because "self" is now freed, and because
self->server_hostname's refcount was already dropped in PySSL_dealloc.

So this can be seen either as a use-after-free or as a double free
vulnerability.


Here's a reproducer:

--- begin script ---

import ssl, socket, _socket

s = ssl.wrap_socket(socket.socket(socket.AF_INET, socket.SOCK_STREAM))
s.context._wrap_socket(_socket.socket(), server_side=1)

--- end script ---

On my machine (Python-3.5.2, 64-bits, --with-pydebug) it crashes:

(gdb) r ./poc8.py
Starting program: /home/xx/Python-3.5.2/python ./poc8.py

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff67f7d9c in newPySSLSocket (sslctx=sslctx@entry=0x7ffff5ed15f8, sock=sock@entry=0x7ffff7e31dc0,
    socket_type=socket_type@entry=PY_SSL_SERVER, server_hostname=<optimized out>, inbio=inbio@entry=0x0, outbio=outbio@entry=0x0)
    at /home/xx/Python-3.5.2/Modules/_ssl.c:562
562                Py_XDECREF(self->server_hostname);
(gdb) p self->server_hostname
$14 = (PyObject *) 0xdbdbdbdbdbdbdbdb


I believe this should be fixed by simply removing the line
"Py_XDECREF(self->server_hostname);"



While fixing this, you might want to fix another issue in newPySSLSocket which
I'll describe next.

The separate problem lies here:

    if (server_hostname != NULL) {
        hostname = PyUnicode_Decode(server_hostname, strlen(server_hostname),
                                   "idna", "strict");
        if (hostname == NULL) {
            Py_DECREF(self);
            return NULL;
        }
        self->server_hostname = hostname;
    }

As we can see, PyUnicode_Decode is called. If PyUnicode_Decode fails, we call
Py_DECREF(self). However the field self->server_hostname is an uninitialized
variable at this point! So the code in PySSL_dealloc which calls
Py_XDECREF(self->server_hostname) could actually be working with an arbitrary,
uninitialized pointer.

Technically this is a separate vulnerability from the first, but I couldn't find
a way to trigger it other than low-memory situations which aren't very
reliable.

This could be fixed by initializing self->server_hostname to NULL before calling
Py_DECREF(self).

New changeset 98c86d5a6655 by Benjamin Peterson in branch '3.5':
fix corner cases in the management of server_hostname (closes #27773)
https://hg.python.org/cpython/rev/98c86d5a6655

New changeset a8cd67e80ed3 by Benjamin Peterson in branch 'default':
merge 3.5 (#27773)
https://hg.python.org/cpython/rev/a8cd67e80ed3

versions: + Python 3.6, - Python 3.11

messages: + msg405691

components: + Parser

nosy: + python-dev
messages: + msg272830

resolution: fixed
stage: resolved