Excessive Py_XDECREF in the ssl module:

[benjaminp]

BPO	27773

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-08-16.04:56:26.518>
created_at = <Date 2016-08-16.04:53:37.087>
labels = ['interpreter-core', 'type-crash']
title = 'Excessive Py_XDECREF in the ssl module:'
updated_at = <Date 2021-11-04.14:21:14.014>
user = 'https://github.com/benjaminp'

bugs.python.org fields:

activity = <Date 2021-11-04.14:21:14.014>
actor = 'eryksun'
assignee = 'none'
closed = True
closed_date = <Date 2016-08-16.04:56:26.518>
closer = 'python-dev'
components = ['Parser']
creation = <Date 2016-08-16.04:53:37.087>
creator = 'benjamin.peterson'
dependencies = []
files = []
hgrepos = []
issue_num = 27773
keywords = []
message_count = 2.0
messages = ['272829', '272830']
nosy_count = 0.0
nosy_names = []
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue27773'
versions = ['Python 3.6']

[benjaminp]

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).

[python-dev]

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

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