Skip to content

Commit

Permalink
bpo-39382: Avoid dangling object use in abstract_issubclass() (GH-18530)
Browse files Browse the repository at this point in the history
Hold reference of __bases__ tuple until tuple item is done with, because by
dropping the reference the item may be destroyed.
(cherry picked from commit 1c56f8f)

Co-authored-by: Yonatan Goldschmidt <yon.goldschmidt@gmail.com>
  • Loading branch information
miss-islington and Jongy committed Feb 22, 2020
1 parent d6965ff commit 0c1827e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 3 deletions.
21 changes: 21 additions & 0 deletions Lib/test/test_isinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,27 @@ def test_isinstance_recursion_limit(self):
# 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.
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ Karan Goel
Jeroen Van Goey
Christoph Gohlke
Tim Golden
Yonatan Goldschmidt
Mark Gollahon
Guilherme Gonçalves
Tiago Gonçalves
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 9 additions & 3 deletions Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -2336,9 +2336,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;
Expand All @@ -2352,7 +2359,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++) {
Expand Down

0 comments on commit 0c1827e

Please sign in to comment.