On 01.09.2015 04:38, Nathaniel Smith wrote:
> Mark Lemburg wrote:
>> Python code will generally assume that it can trust
>> builtin types. It doesn't expect 42 + 2 to clear out the root dir,
>> just because some package installed from PyPI happens to feel in the
>> mood for Easter eggs :-)
> 
> The only reason that'd be possible though is because you went and ran some untrusted code with permissions allowing it to clear out the root dir -- the only way to set up this "exploit" is to run untrusted Python code. Basically you've handed someone a gun, and now you're worried because this patch gives them a new and particularly rube-goldbergian method for pulling the trigger...

I think you're being overly optimistic here. People run unchecked and
unverified code all the time; that's not the same as untrusted, since
trust develops with time and doesn't get reset with each new package
release.

Regardless, the above was only an example. The much more likely thing
to happen is that some code replaces the .__class__ of some unrelated
object by accident via a bug and causes all hell to break loose.

> Except it isn't even a new method; your nasty PyPI package can trivially implement this "easter egg" using only fully-supported features from the stdlib, in any version of Python since 2.5. Here's some nice portable code to do __class__ assignment while dodging *all* the checks in object_set_class:
> 
>   from ctypes import *
>   def set_class(obj, new_class):
>       ob_type_offset = object.__basicsize__ - sizeof(c_void_p)
>       c_void_p.from_address(id(obj) + ob_type_offset).value = id(new_class)
> 
> I mean, obviously ctypes is nasty and breaks the rules, I'm not saying this justifies making __class__ assignment broken as well. But this bug is no more a *security* problem than the existence of ctypes is.

You can disable ctypes easily. OTOH, your patch is inherently changing the
language and making it less secure. There's no way to disable it or
even prevent using it from inside Python. IMO, that's a huge difference.

I also believe that the overall approach is wrong: if you want to add a
feature to Python module objects, please stick to those instead of
changing the overall interpreter semantics.

Some more background:

Replacing .__class__ of class instances is a known and useful Python
feature. However, in the past this was only possible for regular instances,
not for types. With new style classes, this differentiation got
blurred and in Python 3 we only have new style classes, so it may look
like we always wanted this feature to be available. Yet, I'm not sure
whether this was ever intended, or a conscious design decision and
because it creates serious problems for the interpreter, it
definitely needs go through a PEP process first to make everyone
aware of the consequences.