This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Pathological case segmentation fault in issubclass
Type: Stage:
Components: Interpreter Core Versions: Python 2.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, omnifarious, tim.peters
Priority: normal Keywords:

Created on 2003-12-11 03:13 by omnifarious, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
flat_tuple.diff brett.cannon, 2003-12-18 21:39 Requires tuples for isinstance and issubclass to be flat
recursion_check.diff brett.cannon, 2004-03-20 16:14 Checks recursion depth; diff against 2.3 branch
Messages (13)
msg19382 - (view) Author: Eric M. Hopper (omnifarious) Date: 2003-12-11 03:13
This works for the PowerPC Python compiled with gcc 3.3
on OS X using fink.  I suspect it's broader based than
that, but I don't have the ability to check properly.

Here's how to make it segment fault:

x = (basestring,)
for i in xrange(0, 1000000):
   x = (x,)
issubclass(str, x)

At least, it segment faults at the interactive prompt
this way.  I don't know if it does when it's executed
from a file.
msg19383 - (view) Author: Eric M. Hopper (omnifarious) Date: 2003-12-11 03:16
Logged In: YES 
user_id=313

I forgot this:

Python 2.3.2 (#1, Dec  4 2003, 09:13:58) 
[GCC 3.3 20030304 (Apple Computer, Inc. build 1493)] on darwin
Type "help", "copyright", "credits" or "license" for more
information.
msg19384 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2003-12-11 04:28
Logged In: YES 
user_id=357491

If you look at Object/abstract.c (line 2119 or so) for 2.4 CVS you 
will notice that PyObject_IsSubclass goes into a 'for' loop for each 
item in the tuple passed in and calls PyObject_IsSubclass .  
Unfortunately it makes no check for whether the argument it is 
passing is a class itself or not.  This allows it to keep making calls 
as long as the second argument is either a class or a tuple.  This 
is what is leads to the stack being blown and the subsequent 
segfault.

Obvious solution is to put in a check that the argument about to be 
passed is a class itself so as to not have such a deep call chain.  
But since ``help(issubclass)`` actually makes the above use legit 
(it says using a tuple as a second argument is equivalent as 
passing each item to issubclass which is what it is doing, albeit in 
a rather uncommon and pointless way), is it worth putting the 
check in?  Since this is such an obvious mis-use, I say no.  But if 
someone else on python-dev steps in and says otherwise I will 
patch it.
msg19385 - (view) Author: Eric M. Hopper (omnifarious) Date: 2003-12-11 17:54
Logged In: YES 
user_id=313

Well, I think any case where the system segment faults
unexpectedly is bad, regardless of how pathological it is.

Personally, I think that issubclass should either have a
recursion limit after which it throws an exception, or it
shouldn't go into sub-tuples at all.

The reason I made this test is that I read the description
of the behavior of issublcass and found it rather strange,
so I decided to push it to see how far it would go.
msg19386 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-12-14 01:08
Logged In: YES 
user_id=31435

Yes, this needs to be fixed if it *can* be fixed without heroic 
effort or insane slowdown.  Looks like it can be.

Brett, the missing piece of your worldview <wink> here is that 
anywhere Python can be tricked into segfaulting is a kind 
of "security hole" -- it's not just mistakes we want to protect 
programmers from, we also want to bulletproof against hostile 
users, to the extent sanely possible.

BTW, if issubclass() has this insecurity, I bet isinstance() 
does too (they were introduced & coded at the same time).
msg19387 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2003-12-14 20:55
Logged In: YES 
user_id=357491

OK, consider my worldview fixed.  =)

I will add a check in the tuple unpacking 'for' loop to make sure it 
is only passing issubclass classes and not more tuples.  Simple 
and shouldn't break very much code.  Otherwise the code would 
have to keep a count and extra bookkeeping and it would get 
messy quickly.

And I will take a look at isinstance, although this tuple feature was 
added in 2.3 for issubclass so it might not be an issue.

And I will backport it.
msg19388 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-12-14 21:09
Logged In: YES 
user_id=31435

I'm afraid that changing semantics needs to be run through 
the python-dev wringer first -- "shouldn't break very much 
code" isn't "shouldn't break any code".  The *comments* in 
these functions make it appear that they never intended to 
support the OP's original code snippet, but the docs don't 
match.  This leaves the intent a mystery, so it needs to be 
discussed.
msg19389 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2003-12-18 21:39
Logged In: YES 
user_id=357491

I have appended a file that adds a basic test to make sure that 
when the items of a tuple are used to call isinstance or issubclass 
that only classes or types are used; everything else raises 
TypeError.  Also tried to clarify the wording of the doc strings.  
Changed the docs for isinstance since it allowed nested tupling 
while issubclass' didn't.

Have a look and if someone will sign off on it I will apply the patch 
and then start working on a 2.3 solution that doesn't break 
semantics.

And I just realized I left out a Misc/NEWS in the patch; it will be 
there when it gets applied.
msg19390 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-12-18 21:48
Logged In: YES 
user_id=31435

Oh, Brett, you're missing a chance for some fun here!  A bug 
should always be assigned to the next person who should "do 
something" about it.  Think of it as a hot potato.  You should 
assign the bug to who you *want* to review this, and then 
sit back and watch the fun, as each person in turn tries to 
unload the potato onto someone else.  That's one way to get 
a comforting illusion of activity here <wink>.
msg19391 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2003-12-19 05:24
Logged In: YES 
user_id=357491

OK, Tim, start hopping.  =)
msg19392 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2004-02-16 23:44
Logged In: YES 
user_id=31435

Well, that wasn't as much fun for you as I hoped -- the 
report just sat there until I got a free holiday <wink>.  Marked 
Accepted and back to you -- looks good!
msg19393 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2004-03-20 16:14
Logged In: YES 
user_id=357491

Have a patch for Python 2.3 that limits the depth of the tuples to the 
recursion limit of the interpreter.  That will definitely be used for 2.3  
Question is which solution to use for 2.4 .
msg19394 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2004-03-20 23:02
Logged In: YES 
user_id=357491

Fixed in both Python 2.3 and 2.4 so that the depth of the tuple cannot 
exceed the the recursion limit of the interpreter.
History
Date User Action Args
2022-04-11 14:56:01adminsetgithub: 39695
2003-12-11 03:13:04omnifariouscreate