-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-144475: Fix a heap buffer overflow in partial_repr #145362
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
Changes from 1 commit
7a5456c
7b4955f
543d44c
74b98ca
ef0ffb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| Fixed a bug in :func:`functools.partial` when calling :func:`repr` on a partial | ||
| object that could occur when the ``fn``, ``args``, or ``kw`` arguments are modified | ||
| during a call to :func:`repr`. Now, calls to :func:`repr` will use the original | ||
| arguments when generating the string representation of the partial object. | ||
| Subsequent calls will use the updated arguments instead. | ||
|
|
||
|
brijkapadia marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -688,6 +688,7 @@ partial_repr(PyObject *self) | |
| { | ||
| partialobject *pto = partialobject_CAST(self); | ||
| PyObject *result = NULL; | ||
| PyObject *fn, *args, *kw; | ||
| PyObject *arglist; | ||
| PyObject *mod; | ||
| PyObject *name; | ||
|
|
@@ -697,56 +698,65 @@ partial_repr(PyObject *self) | |
|
|
||
| status = Py_ReprEnter(self); | ||
| if (status != 0) { | ||
| if (status < 0) | ||
| if (status < 0) { | ||
| return NULL; | ||
| } | ||
| return PyUnicode_FromString("..."); | ||
| } | ||
| /* Reference arguments in case they change */ | ||
| fn = Py_NewRef(pto->fn); | ||
|
brijkapadia marked this conversation as resolved.
Outdated
|
||
| args = Py_NewRef(pto->args); | ||
| kw = Py_NewRef(pto->kw); | ||
| assert(PyTuple_Check(args)); | ||
| assert(PyDict_Check(kw)); | ||
|
|
||
| arglist = Py_GetConstant(Py_CONSTANT_EMPTY_STR); | ||
| if (arglist == NULL) | ||
| goto done; | ||
| if (arglist == NULL) { | ||
| goto arglist_error; | ||
| } | ||
| /* Pack positional arguments */ | ||
| assert(PyTuple_Check(pto->args)); | ||
| n = PyTuple_GET_SIZE(pto->args); | ||
| n = PyTuple_GET_SIZE(args); | ||
| for (i = 0; i < n; i++) { | ||
| Py_SETREF(arglist, PyUnicode_FromFormat("%U, %R", arglist, | ||
| PyTuple_GET_ITEM(pto->args, i))); | ||
| if (arglist == NULL) | ||
| goto done; | ||
| PyTuple_GET_ITEM(args, i))); | ||
| if (arglist == NULL) { | ||
| goto arglist_error; | ||
| } | ||
| } | ||
| /* Pack keyword arguments */ | ||
| assert (PyDict_Check(pto->kw)); | ||
| for (i = 0; PyDict_Next(pto->kw, &i, &key, &value);) { | ||
| for (i = 0; PyDict_Next(kw, &i, &key, &value);) { | ||
| /* Prevent key.__str__ from deleting the value. */ | ||
| Py_INCREF(value); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're covering all the bases: Here, Also, the iteration should have a critical section around it -- see PyDict_Next docs. But perhaps the best way to solve that would be switching to always use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that adding a critical section here is best for a future PR. This PR is more about fixing a mutation during
I agree, enforcing that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
OK!
The former seems worth looking into. As you said, there's a lot of
Yes. |
||
| Py_SETREF(arglist, PyUnicode_FromFormat("%U, %S=%R", arglist, | ||
| key, value)); | ||
| Py_DECREF(value); | ||
| if (arglist == NULL) | ||
| goto done; | ||
| if (arglist == NULL) { | ||
| goto arglist_error; | ||
| } | ||
| } | ||
|
|
||
| mod = PyType_GetModuleName(Py_TYPE(pto)); | ||
| if (mod == NULL) { | ||
| goto error; | ||
| goto mod_error; | ||
| } | ||
|
|
||
| name = PyType_GetQualName(Py_TYPE(pto)); | ||
| if (name == NULL) { | ||
| Py_DECREF(mod); | ||
| goto error; | ||
| goto name_error; | ||
| } | ||
| result = PyUnicode_FromFormat("%S.%S(%R%U)", mod, name, pto->fn, arglist); | ||
| Py_DECREF(mod); | ||
|
|
||
| result = PyUnicode_FromFormat("%S.%S(%R%U)", mod, name, fn, arglist); | ||
| Py_DECREF(name); | ||
| name_error: | ||
| Py_DECREF(mod); | ||
| mod_error: | ||
| Py_DECREF(arglist); | ||
|
|
||
| done: | ||
| arglist_error: | ||
| Py_DECREF(fn); | ||
| Py_DECREF(args); | ||
| Py_DECREF(kw); | ||
| Py_ReprLeave(self); | ||
|
brijkapadia marked this conversation as resolved.
Outdated
|
||
| return result; | ||
| error: | ||
| Py_DECREF(arglist); | ||
| Py_ReprLeave(self); | ||
| return NULL; | ||
| } | ||
|
|
||
| /* Pickle strategy: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.