diff options
author | Yonatan Goldschmidt <yon.goldschmidt@gmail.com> | 2020-02-22 15:11:48 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-02-22 15:11:48 +0200 |
commit | 1c56f8ffad44478b4214a2bf8eb7cf51c28a347a (patch) | |
tree | c2303c03737343e14f7720a97200bb007845ac7b | |
parent | a025d4ca99fb4c652465368e0b4eb03cf4b316b9 (diff) | |
download | cpython-git-1c56f8ffad44478b4214a2bf8eb7cf51c28a347a.tar.gz |
bpo-39382: Avoid dangling object use in abstract_issubclass() (GH-18530)
Hold reference of __bases__ tuple until tuple item is done with, because by
dropping the reference the item may be destroyed.
-rw-r--r-- | Lib/test/test_isinstance.py | 21 | ||||
-rw-r--r-- | Misc/ACKS | 1 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2020-02-18-01-40-13.bpo-39382.OLSJu9.rst | 3 | ||||
-rw-r--r-- | Objects/abstract.c | 12 |
4 files changed, 34 insertions, 3 deletions
diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index 65751ab916..53639e984e 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -251,6 +251,27 @@ class TestIsInstanceIsSubclass(unittest.TestCase): # blown self.assertRaises(RecursionError, blowstack, isinstance, '', str) + def test_issubclass_refcount_handling(self): + # bpo-39382: abstract_issubclass() didn't hold item reference while + # peeking in the bases tuple, in the single inheritance case. + class A: + @property + def __bases__(self): + return (int, ) + + class B: + def __init__(self): + # setting this here increases the chances of exhibiting the bug, + # probably due to memory layout changes. + self.x = 1 + + @property + def __bases__(self): + return (A(), ) + + self.assertEqual(True, issubclass(B(), int)) + + def blowstack(fxn, arg, compare_to): # Make sure that calling isinstance with a deeply nested tuple for its # argument will raise RecursionError eventually. @@ -594,6 +594,7 @@ Karan Goel Jeroen Van Goey Christoph Gohlke Tim Golden +Yonatan Goldschmidt Mark Gollahon Guilherme Gonçalves Tiago Gonçalves diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-02-18-01-40-13.bpo-39382.OLSJu9.rst b/Misc/NEWS.d/next/Core and Builtins/2020-02-18-01-40-13.bpo-39382.OLSJu9.rst new file mode 100644 index 0000000000..605f4c8e5d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-02-18-01-40-13.bpo-39382.OLSJu9.rst @@ -0,0 +1,3 @@ +Fix a use-after-free in the single inheritance path of ``issubclass()``, when +the ``__bases__`` of an object has a single reference, and so does its first item. +Patch by Yonatan Goldschmidt. diff --git a/Objects/abstract.c b/Objects/abstract.c index f0e01f7691..de5652f3e6 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2379,9 +2379,16 @@ abstract_issubclass(PyObject *derived, PyObject *cls) int r = 0; while (1) { - if (derived == cls) + if (derived == cls) { + Py_XDECREF(bases); /* See below comment */ return 1; - bases = abstract_get_bases(derived); + } + /* Use XSETREF to drop bases reference *after* finishing with + derived; bases might be the only reference to it. + XSETREF is used instead of SETREF, because bases is NULL on the + first iteration of the loop. + */ + Py_XSETREF(bases, abstract_get_bases(derived)); if (bases == NULL) { if (PyErr_Occurred()) return -1; @@ -2395,7 +2402,6 @@ abstract_issubclass(PyObject *derived, PyObject *cls) /* Avoid recursivity in the single inheritance case */ if (n == 1) { derived = PyTuple_GET_ITEM(bases, 0); - Py_DECREF(bases); continue; } for (i = 0; i < n; i++) { |