Skip to content

[3.8] bpo-39382: Avoid dangling object use in abstract_issubclass() (GH-18530) #18607

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

Merged
merged 1 commit into from
Feb 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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