Add some non-NULL assertion checks in `Modules/_ctypes/_ctypes.c` · Issue #131181 · python/cpython · GitHub

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

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

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.

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

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

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

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.

@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())

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.

The analyzer has missed an invariant. Please don't call it a bug.

@ZeroIntensity

if someone manually overwrote the dictionary

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

@encukou , OK, however, adding assert is necessary, right?

It is not. Asserts don't do anything, except in debug builds.

Considering that Petr accepted the merge, I'll remove the pending label.

[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…

Add some non-NULL assertion checks in `Modules/_ctypes/_ctypes.c` · Issue #131181 · python/cpython · GitHub

Repository navigation

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

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	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;
	}

Add some non-NULL assertion checks in `Modules/_ctypes/_ctypes.c` · Issue #131181 · python/cpython · GitHub

Repository navigation

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

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

ZeroIntensity commented on Mar 13, 2025

picnixz commented on Mar 13, 2025

picnixz commented on Mar 13, 2025

ZeroIntensity commented on Mar 13, 2025

encukou commented on Mar 13, 2025

dyupina commented on Mar 13, 2025

picnixz commented on Mar 13, 2025

encukou commented on Mar 13, 2025

dyupina commented on Mar 13, 2025

encukou commented on Mar 13, 2025

picnixz commented on Mar 13, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions