In the source file mmapmodule.c, the function mmap_subscript contains a potential bug about the use of uninitialised variable.


mmapmodule.c:

764 static PyObject *
765 mmap_subscript(mmap_object *self, PyObject *item)
766 {
...
    else if (PySlice_Check(item)) {
782        Py_ssize_t start, stop, step, slicelen;
783
784        if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
785            return NULL;
786        }
787        slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
     ...

In Line 782 of the file mmapmodule.c, the variable stop is not initialised and will be passed to the function PySlice_Unpack as the third parameter. Inside the function, it is likely that stop is not initialised. Please see the following code. 

sliceobject.c:
196 int
197 PySlice_Unpack(PyObject *_r,
198               Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t *step)
199 {
...
231    if (r->stop == Py_None) {
232        *stop = *step < 0 ? PY_SSIZE_T_MIN : PY_SSIZE_T_MAX;
233    }
234    else {
235        if (!_PyEval_SliceIndex(r->stop, stop)) return -1;
236    }


The third parameter **stop** may be changed at line 232 or 235. However, at Line 235, it is still likely that **stop** is not initialised at Line 235 where **stop** is passed as the second parameter. Note that, at Line 235, we only know r->stop!=Py_None. The following is the code snippet of the function _PyEval_SliceIndex.


ceval.c:
4718 int
4719 _PyEval_SliceIndex(PyObject *v, Py_ssize_t *pi)
4720 {
4721     if (v != Py_None) {
4722         Py_ssize_t x;
4723         if (PyIndex_Check(v)) {
4724             x = PyNumber_AsSsize_t(v, NULL);
4725             if (x == -1 && PyErr_Occurred())
4726                 return 0;
4727         }
4728         else {
4729             PyErr_SetString(PyExc_TypeError,
4730                             "slice indices must be integers or "
4731                             "None or have an __index__ method");
4732             return 0;
4733         }
4734         *pi = x;
4735     }
4736    return 1;
4737 }

As we can see, it is likely that when the third parameter v can be NULL, then the function _PyEval_SliceIndex will return 1. In the caller function PySlice_Unpack, at Line 235, the condition **if (!_PyEval_SliceIndex(r->stop, stop))** is not satisfied, and thus it will go to Line 238 which returns 0. In the caller function mmap_subscript in the file mmapmodule.c, at Line 784, since the return value is 0, and thus the path condition **PySlice_Unpack(item, &start, &stop, &step) < 0** is not satisfied. It will continue to execute the Line 787. The uninitialised variable **stop** again will be passed to the function PySlice_AdjustIndices as the third parameter. **stop** then will be dereferenced without initialisation. Please see the following.

sliceobject.c:
241 Py_ssize_t
242 PySlice_AdjustIndices(Py_ssize_t length,
243                      Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t step)
...
260 if (*stop < 0) {
261    *stop += length;
...

BTW, if this bug is true, there is a similar code snippet in the same file.
mmapmodule.c:
845 static int
846 mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
847 {
...
888    else if (PySlice_Check(item)) {
889        Py_ssize_t start, stop, step, slicelen;
890        Py_buffer vbuf;
891
892        if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
893            return -1;
894        }
895        slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);

Your analysis would be (almost) correct if a slice object could have a stop value of NULL. It's wrong in that the error would be a NULL deference, not a silent use of an uninitialized value, but it would be a bug. In your scenario where v == NULL, it would pass the test for v != Py_None, then call PyIndex_Check(v), and since the macro doesn't check for the passed value being NULL, it would perform a NULL deference.

But even that's not possible; PySlice_New (which is ultimately responsible for all slice construction) explicitly replaces any argument of NULL with Py_None, so there is no such thing as a slice with *any* value being NULL.

So since r->stop is definitely non-NULL, either:

1. It's None, PySlice_Unpack line 232 executes, and stop is initialized

or

2. It's non-None, _PyEval_SliceIndex is called with a v that is definitely not None and non-NULL, so it always enters the `if (v != Py_None) {` block, and either it received a value index integer, in which case it initializes *pi (aka stop) and returns 1 (success), or returns 0 (failure), which means stop is never used.

The only way you could trigger your bug is to make a slice with an actual NULL for its stop value (and as noted, the bug would be a NULL dereference in PyIndex_Check, not a use of an uninitialized value, because v != Py_None would return true for v == NULL), which is only possible through intentionally misusing PySliceObject (reaching in and tweaking values of the struct directly). And if you can do that, you're already a C extension (or ctypes code) and can crash the interpreter any number of ways without resorting to this level of complexity.

Hi, Josh Rosenberg. As you mentioned PySlice_New (which is ultimately responsible for all slice construction) explicitly replaces any argument of NULL with Py_None, I am not sure whether this is always true r->stop cannot be NULL. I detected this bug using the code of Python 2.7.15. The implementation of _PyEval_SliceIndex is shown as followings. Please read the comments of this functions, and it is possible that v would be NULL. I wonder whether your assumption r->stop cannot be NULL will be broken in Python 2.7.15



/* Extract a slice index from a PyInt or PyLong or an object with the
   nb_index slot defined, and store in *pi.
   Silently reduce values larger than PY_SSIZE_T_MAX to PY_SSIZE_T_MAX,
   and silently boost values less than PY_SSIZE_T_MIN to PY_SSIZE_T_MIN.
   Return 0 on error, 1 on success.
*/
/* Note:  If v is NULL, return success without storing into *pi.  This
   is because_PyEval_SliceIndex() is called by apply_slice(), which can be
   called by the SLICE opcode with v and/or w equal to NULL.
*/
int
_PyEval_SliceIndex(PyObject *v, Py_ssize_t *pi)
{
    if (v != NULL && v != Py_None) {
        Py_ssize_t x;
        if (PyInt_Check(v)) {
            /* XXX(nnorwitz): I think PyInt_AS_LONG is correct,
               however, it looks like it should be AsSsize_t.
               There should be a comment here explaining why.
            */
            x = PyInt_AS_LONG(v);
        }
        else if (PyIndex_Check(v)) {
            x = PyNumber_AsSsize_t(v, NULL);
            if (x == -1 && PyErr_Occurred())
                return 0;
        }
        else {
            PyErr_SetString(PyExc_TypeError,
                            "slice indices must be integers or "
                            "None or have an __index__ method");
            return 0;
        }
        *pi = x;
    }
    return 1;
}

I think this bug is valid at least in Python 2.7, as I mentioned the implementation of _PyEval_SliceIndex is different from the one in Python 3.8. The condition " if (v != NULL && v != Py_None) " will bypass the NULL pointer dereference. Would you please check this again? Thanks.

Yes, the 2.7 version of _PyEval_SliceIndex would bypass the NULL pointer dereference, so *if* you could make a slice with a NULL stop value, you could trigger a read from uninitialized stack memory, rather than dying due to a NULL pointer dereference.

But just like Python 3, 2.7's PySlice_New explicitly replaces all NULLs with None ( https://github.com/python/cpython/blob/2.7/Objects/sliceobject.c#L60 ), so such a slice cannot exist.

Since you can't make a slice with a NULL value through any supported API, and any unsupported means of doing this means you already have the ability to execute arbitrary code (and do far worse things that just trigger a read from an uninitialized C stack value), the fact that _PyEval_SliceIndex returns success for v == NULL is irrelevant; v isn't NULL in any code path aside of the specific one documented (the SLICE opcode, gone in Py3, which can pass in NULL, but uses defaults of 0 and PY_SSIZE_T_MAX for low and high respectively, so the silent success just leaves the reasonable defaults set), because all other uses use slice objects as the source for v, and they cannot have NULL values.

One additional note, just in case you're wondering. slice explicitly does not set Py_TPFLAGS_BASETYPE (in either Py2 or Py3), so you can't make a subclass of slice with NULLable fields by accident (you'll get a TypeError the moment you try to define it). There is one, and only one, slice type, and its fields are never NULL.

Josh Rosenberg, thanks for your useful comments.