-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
gh-142734: fix UAF in OrderedDict.copy with concurrent mutations by __getitem__
#143059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
OrderedDict.copy with concurrent mutations by __getitem__
picnixz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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