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.
diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py
index 65751ab..53639e9 100644
--- a/Lib/test/test_isinstance.py
+++ b/Lib/test/test_isinstance.py
@@ -251,6 +251,27 @@
# 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.
diff --git a/Misc/ACKS b/Misc/ACKS
index f329a2d..fe24a56 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -594,6 +594,7 @@
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 0000000..605f4c8
--- /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 f0e01f7..de5652f 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -2379,9 +2379,16 @@
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 @@
/* 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++) {