Skip to content

Copy-paste bug in init_containerlistchange + unchecked PyModule_AddObject x10 + cached_property stores NULL #261

@devdanzin

Description

@devdanzin

Three smaller bugs:

1. Copy-paste bug in init_containerlistchange (atomlist.cpp:552-553)

Creates PySStr::__delitem__str but checks PySStr::typestr (the previous variable):

PySStr::__delitem__str = PyUnicode_InternFromString( "__delitem__" );
if( !PySStr::typestr )  // BUG: should be !PySStr::__delitem__str

If __delitem__str creation fails, the error is silently ignored and a NULL pointer propagates.

Fix: check the correct variable:

if( !PySStr::__delitem__str )

2. Unchecked PyModule_AddObject for 10 enum types (catommodule.cpp:151-160)

Return values ignored for all 10 PyModule_AddObject calls for enum types (GetAttr, SetAttr, etc.). On failure, the manually cppy::incref()'d references are leaked and the module loads in a broken state. The other 9 PyModule_AddObject calls (lines 79-135) correctly check return values.

Fix: either check return values and return false on failure, or replace with PyModule_AddObjectRef (available since 3.10):

if( PyModule_AddObjectRef( mod, "GetAttr", PyGetAttr ) < 0 )
    return false;

3. cached_property_handler stores NULL in slot (getattrbehavior.cpp:185-186)

If property_handler fails (returns NULL), set_slot(member->index, value.get()) stores NULL in the atom's slot. Subsequent reads from the cached property slot return NULL, masking the original error.

Fix: check for NULL before storing:

value = property_handler( member, atom );
if( !value )
    return 0;
atom->set_slot( member->index, value.get() );
return value.release();

Found by cext-review-toolkit.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions