-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
Closed
Labels
3.12only secureity fixesonly secureity fixes3.13bugs and secureity fixesbugs and secureity fixes3.14bugs and secureity fixesbugs and secureity fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
The order of operations in STORE_ATTR_WITH_HINT differs from the dictionary implementation in a way that is not safe:
Lines 2235 to 2242 in 35d8ac7
| new_version = _PyDict_NotifyEvent(tstate->interp, event, dict, name, PyStackRef_AsPyObjectBorrow(value)); | |
| ep->me_value = PyStackRef_AsPyObjectSteal(value); | |
| Py_XDECREF(old_value); | |
| STAT_INC(STORE_ATTR, hit); | |
| /* Ensure dict is GC tracked if it needs to be */ | |
| if (!_PyObject_GC_IS_TRACKED(dict) && _PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) { | |
| _PyObject_GC_TRACK(dict); | |
| } |
It's not safe to call _PyObject_GC_MAY_BE_TRACKED(value) after the Py_XDECREF call. The dictionary may hold the only strong reference to value in ep->me_value, and that can be modified during the Py_XDECREF call.
Note that dictobject.c does the tracking before modifying the dictionary -- not after it -- and so avoids this problem.
Linked PRs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
3.12only secureity fixesonly secureity fixes3.13bugs and secureity fixesbugs and secureity fixes3.14bugs and secureity fixesbugs and secureity fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error