Skip to content

Commit 7a5456c

Browse files
committed
gh-144475: Fix a heap buffer overflow in partial_repr (v2)
1 parent 61ebb99 commit 7a5456c

File tree

3 files changed

+91
-23
lines changed

3 files changed

+91
-23
lines changed

Lib/test/test_functools.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,58 @@ def test_partial_genericalias(self):
514514
self.assertEqual(alias.__args__, (int,))
515515
self.assertEqual(alias.__parameters__, ())
516516

517+
# Issue 144475
518+
def test_repr_saftey_against_reentrant_mutation(self):
519+
g_partial = None
520+
521+
class Function:
522+
def __init__(self, name):
523+
self.name = name
524+
525+
def __call__(self):
526+
return None
527+
528+
def __repr__(self):
529+
return f"Function({self.name})"
530+
531+
class EvilObject:
532+
def __init__(self):
533+
self.triggered = False
534+
535+
def __repr__(self):
536+
if not self.triggered and g_partial is not None:
537+
self.triggered = True
538+
new_args_tuple = (None,)
539+
new_keywords_dict = {"keyword": None}
540+
new_tuple_state = (Function("new_function"), new_args_tuple, new_keywords_dict, None)
541+
g_partial.__setstate__(new_tuple_state)
542+
gc.collect()
543+
return f"EvilObject"
544+
545+
trigger = EvilObject()
546+
func = Function("old_function")
547+
548+
g_partial = functools.partial(func, None, trigger=trigger)
549+
self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), None, trigger=EvilObject)")
550+
551+
trigger.triggered = False
552+
g_partial = functools.partial(func, trigger, arg=None)
553+
self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject, arg=None)")
554+
555+
556+
trigger.triggered = False
557+
g_partial = functools.partial(func, trigger, None)
558+
self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject, None)")
559+
560+
trigger.triggered = False
561+
g_partial = functools.partial(func, trigger=trigger, arg=None)
562+
self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), trigger=EvilObject, arg=None)")
563+
564+
trigger.triggered = False
565+
g_partial = functools.partial(func, trigger, None, None, None, None, arg=None)
566+
self.assertEqual(repr(g_partial),"functools.partial(Function(old_function), EvilObject, None, None, None, None, arg=None)")
567+
568+
517569

518570
@unittest.skipUnless(c_functools, 'requires the C _functools module')
519571
class TestPartialC(TestPartial, unittest.TestCase):
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fixed a bug in :func:`functools.partial` when calling :func:`repr` on a partial
2+
object that could occur when the ``fn``, ``args``, or ``kw`` arguments are modified
3+
during a call to :func:`repr`. Now, calls to :func:`repr` will use the original
4+
arguments when generating the string representation of the partial object.
5+
Subsequent calls will use the updated arguments instead.
6+

Modules/_functoolsmodule.c

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ partial_repr(PyObject *self)
688688
{
689689
partialobject *pto = partialobject_CAST(self);
690690
PyObject *result = NULL;
691+
PyObject *fn, *args, *kw;
691692
PyObject *arglist;
692693
PyObject *mod;
693694
PyObject *name;
@@ -697,56 +698,65 @@ partial_repr(PyObject *self)
697698

698699
status = Py_ReprEnter(self);
699700
if (status != 0) {
700-
if (status < 0)
701+
if (status < 0) {
701702
return NULL;
703+
}
702704
return PyUnicode_FromString("...");
703705
}
706+
/* Reference arguments in case they change */
707+
fn = Py_NewRef(pto->fn);
708+
args = Py_NewRef(pto->args);
709+
kw = Py_NewRef(pto->kw);
710+
assert(PyTuple_Check(args));
711+
assert(PyDict_Check(kw));
704712

705713
arglist = Py_GetConstant(Py_CONSTANT_EMPTY_STR);
706-
if (arglist == NULL)
707-
goto done;
714+
if (arglist == NULL) {
715+
goto arglist_error;
716+
}
708717
/* Pack positional arguments */
709-
assert(PyTuple_Check(pto->args));
710-
n = PyTuple_GET_SIZE(pto->args);
718+
n = PyTuple_GET_SIZE(args);
711719
for (i = 0; i < n; i++) {
712720
Py_SETREF(arglist, PyUnicode_FromFormat("%U, %R", arglist,
713-
PyTuple_GET_ITEM(pto->args, i)));
714-
if (arglist == NULL)
715-
goto done;
721+
PyTuple_GET_ITEM(args, i)));
722+
if (arglist == NULL) {
723+
goto arglist_error;
724+
}
716725
}
717726
/* Pack keyword arguments */
718-
assert (PyDict_Check(pto->kw));
719-
for (i = 0; PyDict_Next(pto->kw, &i, &key, &value);) {
727+
for (i = 0; PyDict_Next(kw, &i, &key, &value);) {
720728
/* Prevent key.__str__ from deleting the value. */
721729
Py_INCREF(value);
722730
Py_SETREF(arglist, PyUnicode_FromFormat("%U, %S=%R", arglist,
723731
key, value));
724732
Py_DECREF(value);
725-
if (arglist == NULL)
726-
goto done;
733+
if (arglist == NULL) {
734+
goto arglist_error;
735+
}
727736
}
728737

729738
mod = PyType_GetModuleName(Py_TYPE(pto));
730739
if (mod == NULL) {
731-
goto error;
740+
goto mod_error;
732741
}
742+
733743
name = PyType_GetQualName(Py_TYPE(pto));
734744
if (name == NULL) {
735-
Py_DECREF(mod);
736-
goto error;
745+
goto name_error;
737746
}
738-
result = PyUnicode_FromFormat("%S.%S(%R%U)", mod, name, pto->fn, arglist);
739-
Py_DECREF(mod);
747+
748+
result = PyUnicode_FromFormat("%S.%S(%R%U)", mod, name, fn, arglist);
740749
Py_DECREF(name);
750+
name_error:
751+
Py_DECREF(mod);
752+
mod_error:
741753
Py_DECREF(arglist);
742-
743-
done:
754+
arglist_error:
755+
Py_DECREF(fn);
756+
Py_DECREF(args);
757+
Py_DECREF(kw);
744758
Py_ReprLeave(self);
745759
return result;
746-
error:
747-
Py_DECREF(arglist);
748-
Py_ReprLeave(self);
749-
return NULL;
750760
}
751761

752762
/* Pickle strategy:

0 commit comments

Comments
 (0)