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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions