Repository navigation
Add some non-NULL assertion checks in Modules/_ctypes/_ctypes.c #131181
Description
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
Activity
ZeroIntensity commentedon Mar 13, 2025
That doesn't sound particularly useful. We should just add a
if (dict == NULL)check and return theMemoryError. PR welcome :)picnixz commentedon Mar 13, 2025
We want to be as fast as possible I think, but I guess an assertion doesn't hurt.
I think that by construction, we can prove that the
PyType_stgdict(item_type)is non-NULL but I'm not entirely sure. BecausePyCData_item_type()asserts thatPyType_stgdictis non-NULL.picnixz commentedon Mar 13, 2025
It's not creating something new, it's returning a borrowed reference technically.
[-]fix null pointer dereference[/-][+][3.12] Possible NULL pointer dereference in `Modules/_ctypes/_ctypes.c`[/+][-][3.12] Possible NULL pointer dereference in `Modules/_ctypes/_ctypes.c`[/-][+]Possible NULL pointer dereference in `Modules/_ctypes/_ctypes.c`[/+]ZeroIntensity commentedon Mar 13, 2025
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
I guess it's possible for it to return
NULLif someone manually overwrote the dictionary?cc @encukou
encukou commentedon Mar 13, 2025
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 commentedon Mar 13, 2025
@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 ofPyType_stgdict())picnixz commentedon Mar 13, 2025
Ah sorry, I think I got confused with the function. I was looking at the other call:
Here, this type with
item_dict. So it was off-topic my bad.encukou commentedon Mar 13, 2025
The analyzer has missed an invariant. Please don't call it a bug.
@ZeroIntensity
Well, overwriting
__dict__of a ctypes type, or using non-stgdict in a subclass, will get you in trouble. But at that point you're asking for a segfault, and there are much easier ways to get one that with ctypes ;)dyupina commentedon Mar 13, 2025
@encukou , OK, however, adding assert is necessary, right?
encukou commentedon Mar 13, 2025
It is not. Asserts don't do anything, except in debug builds.
picnixz commentedon Mar 13, 2025
Considering that Petr accepted the merge, I'll remove the
pendinglabel.[-]Possible NULL pointer dereference in `Modules/_ctypes/_ctypes.c`[/-][+]Add some non-NULL assertion checks in `Modules/_ctypes/_ctypes.c`[/+][3.12] gh-131181: Assert that PyObject_stgdict isn't NULL for PyCData (…
[3.12] pythongh-131181: Assert that PyObject_stgdict isn't NULL for P…