[ python-Bugs-1153075 ] PyXxx_Check(x) trusts x->ob_type->tp_mro

[SourceForge.net]

Bugs item #1153075, was opened at 2005-02-27 21:55
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1153075&group_id=5470

Category: Python Interpreter Core
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Armin Rigo (arigo)
Assigned to: Nobody/Anonymous (nobody)
Summary: PyXxx_Check(x) trusts x->ob_type->tp_mro

Initial Comment:
The functions PyInt_Check(), PyString_Check(),
PyList_Check() etc. are used all over the core to check
which typecasts are safe, from PyObject* to the various
PyXxxObject*.

But the macros themselves are implemented by
inspecting the "tp_mro" tuple of the incoming object's
type.  As the latter can be completely controlled by the
user, an object can pretend to inherit from anything and
pass the PyXxx_Check() checks of its choice, even if
its memory layout is actually completely wrong.

See attached example.

----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2005-03-03 09:34

Message:
Logged In: YES 
user_id=4771

The PyXxx_Check() macros are #defined with
PyObject_TypeCheck().  Should we change all these #defines to
use a new PyObject_LayoutCheck() or something, and document
to C extension writers that they should also switch to the
new function?  More generally, should we review *all* usages
of PyObject_TypeCheck() and decide if they really meant that
or PyObject_LayoutCheck()?  Do we care about types that are
solid subclasses of 'int' but don't have it in their MRO? 
Should we just forget the whole idea and sanity-check the
user-defined mro, which after all would just add another
strange undocumented condition to typeobject.c? :-)

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2005-03-02 20:11

Message:
Logged In: YES 
user_id=6656

Hmm, yeah, that works.  It wasn't totally clear to me that the user couldn't 
maliciously influence tp_base, but I don't think they can...

A helper function is presumably in order, unless PyType_IsSubtype should 
be changed.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2005-03-02 17:03

Message:
Logged In: YES 
user_id=4771

To solve the problem, as hinted in the title of this tracker, I 
think that PyXxx_Check() should simply not trust any mro.  
What PyInt_Check() really means at the C level is to check if 
an object memory layout is compatible with PyIntObject.  This 
is easy to figure out by walking the "solid base" chain, 
tp_base.

As a side note, PyPy gives the same error as Python 2.2.  
However, both in PyPy and in Python 2.2, you can still build 
an instance of the strange class X as follows:

>>> x = object.__new__(X)

Still, all the methods of x are resolved via the dict type.  In 
PyPy we get a clean TypeError because the methods thus 
found don't apply to non-dict objects.  In Python 2.2 you can 
crash the interpreter just as in more recent Python releases, 
e.g. with x[5]=6.

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2005-03-01 20:18

Message:
Logged In: YES 
user_id=6656

I wonder if Guido owes Armin a beer (read typeobject.c around line 5230).  
Well, not really, but it's that code that is making the difference.

However, reversing the order of dict and object is sufficient to make 2.2 
crash (I presume, it crashes CVS HEAD with the __new__ short-circuiting 
code chopped out which gives the error jelper observes on the original).

I wonder what to do about this.  Removing the ability to customize the mro 
from user code is one obvious approach -- I don't know how much code 
uses this feature though.

----------------------------------------------------------------------

Comment By: Jeff Epler (jepler)
Date: 2005-03-01 19:15

Message:
Logged In: YES 
user_id=2772

Not sure if this is relevant, but the example given didn't
crash 2.2:
$ python2.2 bug.py
Traceback (most recent call last):
  File "bug.py", line 9, in ?
    x = X()
TypeError: dict.__new__(X) is not safe, use object.__new__()

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1153075&group_id=5470