gh-123083: Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT`` (gh-123092)

This commit is contained in:
Donghee Na 2024-08-22 23:49:09 +09:00 committed by GitHub
parent 4abc1c1456
commit 297f2e093e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 48 additions and 24 deletions

View File

@ -1476,6 +1476,24 @@ class DictTest(unittest.TestCase):
gc.collect()
self.assertTrue(gc.is_tracked(next(it)))
def test_store_evilattr(self):
class EvilAttr:
def __init__(self, d):
self.d = d
def __del__(self):
if 'attr' in self.d:
del self.d['attr']
gc.collect()
class Obj:
pass
obj = Obj()
obj.__dict__ = {}
for _ in range(10):
obj.attr = EvilAttr(obj.__dict__)
def test_str_nonstr(self):
# cpython uses a different lookup function if the dict only contains
# `str` keys. Make sure the unoptimized path is used when a non-`str`

View File

@ -0,0 +1 @@
Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT``.

View File

@ -1703,6 +1703,8 @@ insert_split_value(PyInterpreterState *interp, PyDictObject *mp, PyObject *key,
uint64_t new_version = _PyDict_NotifyEvent(interp, PyDict_EVENT_MODIFIED, mp, key, value);
STORE_SPLIT_VALUE(mp, ix, Py_NewRef(value));
mp->ma_version_tag = new_version;
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
// when dict only holds the strong reference to value in ep->me_value.
Py_DECREF(old_value);
}
ASSERT_CONSISTENT(mp);

View File

@ -2235,18 +2235,19 @@ dummy_func(
DEOPT_IF(!DK_IS_UNICODE(dict->ma_keys));
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name);
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
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);
}
/* PEP 509 */
dict->ma_version_tag = new_version;
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict, name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
dict->ma_version_tag = new_version; // PEP 509
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
// when dict only holds the strong reference to value in ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
PyStackRef_CLOSE(owner);
}

View File

@ -2637,18 +2637,19 @@
UOP_STAT_INC(uopcode, miss);
JUMP_TO_JUMP_TARGET();
}
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
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);
}
/* PEP 509 */
dict->ma_version_tag = new_version;
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict, name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
dict->ma_version_tag = new_version; // PEP 509
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
// when dict only holds the strong reference to value in ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
PyStackRef_CLOSE(owner);
stack_pointer += -2;
assert(WITHIN_STACK_BOUNDS());

View File

@ -6917,18 +6917,19 @@
DEOPT_IF(!DK_IS_UNICODE(dict->ma_keys), STORE_ATTR);
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name, STORE_ATTR);
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
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);
}
/* PEP 509 */
dict->ma_version_tag = new_version;
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict, name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
dict->ma_version_tag = new_version; // PEP 509
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
// when dict only holds the strong reference to value in ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
PyStackRef_CLOSE(owner);
}
stack_pointer += -2;