Add some non-NULL assertion checks in `Modules/_ctypes/_ctypes.c`

[dyupina]

Bug report

Bug description:

In the PyCData_NewGetBuffer() (file Modules/_ctypes/_ctypes.c), the PyObject_stgdict() is called, which may return NULL.
Its result is assigned to dict, then dict is dereferenced (view->format = dict->format ? dict->format : "B";).
If dict is NULL, a null pointer dereference will occur, so a check for NULL need to be added.

Similarly, in the PyCData_reduce() (file Modules/_ctypes/_ctypes.c) PyObject_stgdict(myself) may return NULL, so need to check result to prevent null pointer dereferencing.

Commit dcaf33a (Author: @encukou) fixes this problems, but it is very large and probably difficult to backport, so I suggest adding assert(dict); before the dereference.

This needs to be added to branches 3.1 - 3.12.
Found by Linux Verification Center (linuxtesting.org) with SVACE.

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

• gh-131181: Fix possible NULL pointer dereference in Modules/_ctypes/_ctypes.c #131186
• [3.12] gh-131181: Add some non-NULL assertion checks in Modules/_ctypes/_ctypes.c #131188

[ZeroIntensity]

assert(dict); before the dereference.

That doesn't sound particularly useful. We should just add a if (dict == NULL) check and return the MemoryError. PR welcome :)

[picnixz]

If dict is NULL, a null pointer dereference will occur, so a check for NULL need to be added.

We want to be as fast as possible I think, but I guess an assertion doesn't hurt.

Similarly, in the PyCData_reduce() (file Modules/_ctypes/_ctypes.c) PyObject_stgdict(myself) may return NULL, so need to check result to prevent null pointer dereferencing.

I think that by construction, we can prove that the PyType_stgdict(item_type) is non-NULL but I'm not entirely sure. Because PyCData_item_type() asserts that PyType_stgdict is non-NULL.

[picnixz]

if (dict == NULL) check and return the MemoryError

It's not creating something new, it's returning a borrowed reference technically.

[ZeroIntensity]

Oh, interesting! I assumed it made an allocation. (That function doesn't exist on recent versions, don't blame me 😆)

Putting the code here so I don't have to keep going back to it:

cpython/Modules/_ctypes/stgdict.c

Lines 199 to 206 in 969631a

	StgDictObject *
	PyObject_stgdict(PyObject *self)
	{
	PyTypeObject *type = Py_TYPE(self);
	if (!type->tp_dict || !PyCStgDict_CheckExact(type->tp_dict))
	return NULL;
	return (StgDictObject *)type->tp_dict;
	}

I guess it's possible for it to return NULL if someone manually overwrote the dictionary?
cc @encukou

[encukou]

Can you actually reproduce this?

I don't think it's a bug, but, send the PR & I'll merge it.
Making the code easy to read is good, even -- to an extent -- for automated tools.

[dyupina]

@encukou, these bugs were found by a static analyzer and checked analytically, so there are no reproducers for them

@picnixz, sorry, I don't understand your comment about PyCData_reduce() (I don't see any use of PyType_stgdict())

[picnixz]

Ah sorry, I think I got confused with the function. I was looking at the other call:

    PyObject *item_type = PyCData_item_type((PyObject*)Py_TYPE(myself));
    StgDictObject *item_dict = PyType_stgdict(item_type);

Here, this type with item_dict. So it was off-topic my bad.