Unsafe use of PyList_GetItem()?

Python Help
1

In cpython/termios.c at main · python/cpython · GitHub, there is a piece of code:

PyObject *cc = PyList_GetItem(term, 6);
    if (PyErr_Occurred()) {
        return NULL;
    }

    if (!PyList_Check(cc) || PyList_Size(cc) != NCCS) {
        PyErr_Format(PyExc_TypeError,
            "tcsetattr: attributes[6] must be %d element list",
                 NCCS);
        return NULL;
    }

    int i;
    PyObject *v;
    for (i = 0; i < NCCS; i++) {
        v = PyList_GetItem(cc, i);

        if (PyBytes_Check(v) && PyBytes_Size(v) == 1)
            mode.c_cc[i] = (cc_t) * PyBytes_AsString(v);
        else if (PyLong_Check(v))
            mode.c_cc[i] = (cc_t) PyLong_AsLong(v);
        else {
            PyErr_SetString(PyExc_TypeError,
     "tcsetattr: elements of attributes must be characters or integers");
                        return NULL;
                }
    }

Since PyObject *cc = PyList_GetItem(term, 6); returns a borrowed reference, I am wondering if the following code v = PyList_GetItem(cc, i); is safe. Should we add Py_INCREF(cc), before the code, and then Py_DECREF(cc)?

2

or
if (!PyList_Check(cc) || PyList_Size(cc) != NCCS) could ensure the cc is safe to be further used?

3

So long as the list still exists, and retains a reference to that particular element, the borrowed reference will remain valid. There’s no way to call arbitrary Python code between the GetItem and the usage, so it should be safe.

(It IS technically possible for PyLong_AsLong to call arbitrary code, but after the call to PyLong_AsLong, v isn’t used again, so it won’t be a problem.)

4

So the issue lies in whether to ensure the list exists, especially in the case of borrowed reference.

The possible vulnerabilities are:
v = PyList_GetItem(cc, i)
mode.c_cc[i] = (cc_t) * PyBytes_AsString(v)

Here are some questions:
(1) Without calling Py_INCREF(), can we still ensure that the list exists? (2) or by calling if (PyBytes_Check(v) && PyBytes_Size(v) == 1), is further use of v ok? (3) or as you said “it is technically possible for PyLong_AsLong to call arbitrary code, but after the call to PyLong_AsLong, v isn’t used again, so it won’t be a problem.” but PyLong_AsLong still doesn’t look safe to me.

5

Hmm, actually, good point - I slightly misread some of the checks being done here. It turns out, this is actually entirely safe, and my parenthetical comment was unnecessary; while it IS possible for PyLong_AsLong to call arbitrary code, it will not do so if PyLong_Check returns true (which it will do for an int, or any subclass of int). So since this first uses PyLong_Check to determine whether it should be accepting the value, and then uses PyLong_AsLong which uses PyLong_Check to determine whether to call __index__ or to directly parse the value, this code has no way to call arbitrary Python code, and therefore has no way to lose the original reference to term, nor any way to mutate the term list such that it loses the reference to cc.

So, yes, we can be sure that the list still exists, but only because this will ONLY call PyLong_AsLong when it is safe to do so.