gh-142734: fix UAF in OrderedDict.copy with concurrent mutations by __getitem__#143059
gh-142734: fix UAF in OrderedDict.copy with concurrent mutations by __getitem__#143059fatelei wants to merge 9 commits intopython:mainfrom
OrderedDict.copy with concurrent mutations by __getitem__#143059Conversation
OrderedDict.copy with concurrent mutations by __getitem__
picnixz
left a comment
There was a problem hiding this comment.
This implementation is not efficient. I think it's better that we just bail during the copy (that is, raise) if we detect a state change (if possible). We had a similar patch in __eq__.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
update PyODictObject *self = _PyODictObject_CAST(od);
size_t state = self->od_state;
_ODictNode *cur = _odict_FIRST(od);
while (cur != NULL) {
if (self->od_state != state) {
PyErr_SetString(PyExc_RuntimeError,
"OrderedDict mutated during iteration");
goto fail;
}
PyObject *key = Py_NewRef(_odictnode_KEY(cur));
PyObject *value = PyObject_GetItem((PyObject *)od, key);
if (value == NULL) {
Py_DECREF(key);
goto fail;
}
if (self->od_state != state) {
Py_DECREF(key);
Py_DECREF(value);
PyErr_SetString(PyExc_RuntimeError,
"OrderedDict mutated during iteration");
goto fail;
}
if (PyObject_SetItem((PyObject *)od_copy, key, value) != 0) {
Py_DECREF(key);
Py_DECREF(value);
goto fail;
}
Py_DECREF(key);
Py_DECREF(value);
cur = _odictnode_NEXT(cur);
} |
picnixz
left a comment
There was a problem hiding this comment.
Please add tests when the dictionary is shrinked or grows during the iteration (not just cleared)
test update |
fix OrderedDict copy heap-use-after-free security issue
OrderedDict.copyvia re-entrant__getitem__#142734