-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
gh-144475: Fix a heap buffer overflow in partial_repr #144571
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?
Changes from all commits
b0ffe74
7c1ed7e
2723186
239c40d
e53d7e9
661af78
5ce61d9
92eb6ea
5bf71f9
cf63067
838c8b1
cb25ea5
e18f2ad
f1dc06f
30d39f4
a004007
24307a2
192ff1e
715949f
a196de4
ac8ff42
0a102fd
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 |
|---|---|---|
|
|
@@ -514,6 +514,66 @@ def test_partial_genericalias(self): | |
| self.assertEqual(alias.__args__, (int,)) | ||
| self.assertEqual(alias.__parameters__, ()) | ||
|
|
||
| # Issue 144475 | ||
| def test_repr_saftey_against_reentrant_mutation(self): | ||
| g_partial = None | ||
|
|
||
| class Function: | ||
| def __init__(self, name): | ||
| self.name = name | ||
|
|
||
| def __call__(self): | ||
| return None | ||
|
|
||
| def __repr__(self): | ||
| return f"Function({self.name})" | ||
|
|
||
| class EvilObject: | ||
| def __init__(self, name, is_trigger=False): | ||
| self.name = name | ||
| self.is_trigger = is_trigger | ||
| self.triggered = False | ||
|
|
||
| def __repr__(self): | ||
| if self.is_trigger and not self.triggered and g_partial is not None: | ||
| self.triggered = True | ||
| new_args_tuple = (None,) | ||
| new_keywords_dict = {"keyword": None} | ||
| new_tuple_state = (Function("new_function"), new_args_tuple, new_keywords_dict, None) | ||
| g_partial.__setstate__(new_tuple_state) | ||
| gc.collect() | ||
| return f"EvilObject({self.name})" | ||
|
|
||
| trigger = EvilObject("trigger", is_trigger=True) | ||
| victim = EvilObject("victim") | ||
|
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. Why |
||
|
|
||
| g_partial = functools.partial(Function("old_function"), victim, victim=trigger) | ||
| self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject(victim), victim=EvilObject(trigger))") | ||
|
|
||
| trigger.triggered = False | ||
| g_partial = None | ||
| g_partial = functools.partial(Function("old_function"), trigger, victim=victim) | ||
| self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject(trigger), victim=EvilObject(victim))") | ||
|
|
||
|
|
||
| trigger.triggered = False | ||
| g_partial = functools.partial(Function("old_function"), trigger, victim) | ||
| self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject(trigger), EvilObject(victim))") | ||
|
|
||
| trigger.triggered = False | ||
| g_partial = functools.partial(Function("old_function"), trigger=trigger, victim=victim) | ||
| self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), trigger=EvilObject(trigger), victim=EvilObject(victim))") | ||
|
|
||
| trigger.triggered = False | ||
| victim1 = EvilObject("victim") | ||
| victim2 = EvilObject("victim") | ||
| victim3 = EvilObject("victim") | ||
| victim4 = EvilObject("victim") | ||
| victim5 = EvilObject("victim") | ||
| g_partial = functools.partial(Function("old_function"), trigger, victim1, victim2, victim3, victim4, victim=victim5) | ||
| self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject(trigger), EvilObject(victim), EvilObject(victim), EvilObject(victim), EvilObject(victim), victim=EvilObject(victim))") | ||
|
|
||
|
|
||
|
|
||
| @unittest.skipUnless(c_functools, 'requires the C _functools module') | ||
| class TestPartialC(TestPartial, unittest.TestCase): | ||
|
|
||
| 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. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -689,6 +689,9 @@ partial_repr(PyObject *self) | |
| partialobject *pto = partialobject_CAST(self); | ||
| PyObject *result = NULL; | ||
| PyObject *arglist; | ||
| PyObject *fn; | ||
| PyObject *args; | ||
| PyObject *kw; | ||
| PyObject *mod; | ||
| PyObject *name; | ||
| Py_ssize_t i, n; | ||
|
|
@@ -701,52 +704,55 @@ partial_repr(PyObject *self) | |
| return NULL; | ||
| return PyUnicode_FromString("..."); | ||
| } | ||
| /* Reference arguments in case they change */ | ||
| fn = Py_NewRef(pto->fn); | ||
| 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; | ||
| 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))); | ||
| PyTuple_GET_ITEM(args, i))); | ||
| if (arglist == NULL) | ||
| goto done; | ||
| 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); | ||
| Py_SETREF(arglist, PyUnicode_FromFormat("%U, %S=%R", arglist, | ||
| key, value)); | ||
| Py_DECREF(value); | ||
| if (arglist == NULL) | ||
| goto done; | ||
| goto arglist_error; | ||
| } | ||
|
|
||
| mod = PyType_GetModuleName(Py_TYPE(pto)); | ||
| if (mod == NULL) { | ||
| goto error; | ||
| } | ||
| if (mod == NULL) | ||
|
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. PEP 7 requires braces even for single-statement branches in new code. So, please restore the removed braces. |
||
| goto mod_error; | ||
|
|
||
| name = PyType_GetQualName(Py_TYPE(pto)); | ||
| if (name == NULL) { | ||
| Py_DECREF(mod); | ||
| goto error; | ||
| } | ||
| result = PyUnicode_FromFormat("%S.%S(%R%U)", mod, name, pto->fn, arglist); | ||
| Py_DECREF(mod); | ||
| if (name == NULL) | ||
| goto name_error; | ||
|
|
||
| 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); | ||
| return result; | ||
bkap123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| error: | ||
| Py_DECREF(arglist); | ||
| Py_ReprLeave(self); | ||
| return NULL; | ||
| } | ||
|
|
||
| /* Pickle strategy: | ||
|
|
||
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.
Why do we need the Function class and cannot use simple function like
print?