classification
Title: inspect.getattr_static code execution
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: Trundle, daniel.urban, michael.foord, python-dev, segfaulthunter
Priority: normal Keywords: patch

Created on 2011-02-06 16:30 by daniel.urban, last changed 2011-03-16 15:40 by michael.foord. This issue is now closed.

Files
File name Uploaded Description Edit
inspect_issue_11133.patch Trundle, 2011-02-20 17:52 review
inspect_issue_11133_v2.patch Trundle, 2011-02-22 01:42 review
Messages (11)
msg128067 - (view) Author: Daniel Urban (daniel.urban) * Date: 2011-02-06 16:30
The documentation of getattr_static says:
"The only known case that can cause getattr_static to trigger code execution, and cause it to return incorrect results (or even break), is where a class uses __slots__ and provides a __dict__ member using a property or descriptor. If you find other cases please report them so they can be fixed or documented."

I'd like to report another case: when an object's __dict__ is an instance of a dict subclass which overrides dict.get:

>>> _sentinel = object()
>>> 
>>> class MyDict(dict):
...     def get(self, key, default=_sentinel):
...             print('Hello World!') # This code will execute
...             if default is _sentinel:
...                     return super().get(key)
...             else:
...                     return super().get(key, default)
... 
>>> class X:
...     def __init__(self):
...             self.__dict__ = MyDict()
... 
>>> x = X()
>>> inspect.getattr_static(x, 'foo', 0)
Hello World!
0
>>> 

(In line 1072. _check_instance calls MyDict.get: instance_dict.get(attr, _sentinel).)
msg128073 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-02-06 19:34
The fix is to use dict methods rather than accessing members through the instance. It will have to wait until 3.2 is out now though.
msg128904 - (view) Author: Florian Mayer (segfaulthunter) Date: 2011-02-20 12:48
Apparently another way to get getattr_static to execute code in Python 2.3rc3 is simply the following.

>>> class Foo:
...     @property
...     def __dict__(self):
...         print("Hello, World.")
...         return {}
... 
>>> import inspect
>>> inspect.getattr_static(Foo(), 'a')
Hello, World.
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/name/opt/lib/python3.2/inspect.py", line 1130, in getattr_static
    raise AttributeError(attr)
AttributeError: a
>>>
msg128916 - (view) Author: Andreas Stührk (Trundle) Date: 2011-02-20 17:52
Attached is a patch that fixes the issue: The dict methods are now used directly and before every access to an instance's "__dict__" attribute, it is checked that that attribute is really the instance's attribute and not a class attribute of the instance's type.
msg128958 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-02-21 11:36
__dict__ as a property is documented as an exception to the "no code execution" claim.

The patch is not sufficient - instances may have a class member "__dict__" whilst still having an instance __dict__. Alternatively the "__dict__" property may be provided by a base class and so not available in "type(obj).__dict__" but still be provided by a property.

I don't think there is any general way to tell whether fetching obj.__dict__ will get an instance dictionary or fetch a "__dict__" member from the class or a base-class... (Hence the documented exception.)
msg129024 - (view) Author: Andreas Stührk (Trundle) Date: 2011-02-22 01:40
> The patch is not sufficient - instances may have a class member "__dict__" whilst still having an instance __dict__.

Sure, but I don't think there is a way how you can access the instance
__dict__ in that case inside Python code. At least I can't think of
one.

>Alternatively the "__dict__" property may be provided by a base class and so not available in "type(obj).__dict__" but still be provided by a property.
>
> I don't think there is any general way to tell whether fetching obj.__dict__ will get an instance dictionary or fetch a "__dict__" member from the class or a base-class... (Hence the documented exception.)

Why not? ``obj.__dict__`` will fetch the instance dictionary iff there
is no class attribute "__dict__" in any of the base classes. In the
patch,``type.__dict__["__dict__"].__get__()`` is used to get (without
any doubt) the class dictionary. By looking inside that dictionary, we
can now tell whether "__dict__" is overwritten: If it isn't
overwritten, the dictionary either doesn't have a "__dict__" entry at
all or the value is a getset_descriptor. So we just need to iterate
over a type's mro, look inside each entries' dictionary and stop when
a "__dict__" entry is found.
msg129025 - (view) Author: Andreas Stührk (Trundle) Date: 2011-02-22 01:42
Updated patch.
msg131066 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-15 23:19
New changeset 8c7eac34f7bf by Michael Foord in branch '3.2':
Closes issue 11133. Fixes two cases where inspect.getattr_static could trigger code execution
http://hg.python.org/cpython/rev/8c7eac34f7bf
msg131092 - (view) Author: Daniel Urban (daniel.urban) * Date: 2011-03-16 07:43
The new entry in Misc/NEWS says: "Patch by Daniel Urban." But it wasn't me, who made the patch, I just opened the issue.
msg131129 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-16 15:29
New changeset 382cb3386d57 by Benjamin Peterson in branch '3.2':
correct patch ack (#11133)
http://hg.python.org/cpython/rev/382cb3386d57
msg131132 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-03-16 15:40
Thanks Daniel (and sorry Andreas). Benjamin Peterson has fixed this.
History
Date User Action Args
2011-03-16 15:40:15michael.foordsetnosy: segfaulthunter, michael.foord, Trundle, daniel.urban, python-dev
messages: + msg131132
2011-03-16 15:29:46python-devsetnosy: segfaulthunter, michael.foord, Trundle, daniel.urban, python-dev
messages: + msg131129
2011-03-16 07:43:43daniel.urbansetnosy: segfaulthunter, michael.foord, Trundle, daniel.urban, python-dev
messages: + msg131092
2011-03-15 23:19:48python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg131066

resolution: fixed
stage: resolved
2011-02-22 01:42:10Trundlesetfiles: + inspect_issue_11133_v2.patch
nosy: segfaulthunter, michael.foord, Trundle, daniel.urban
messages: + msg129025
2011-02-22 01:40:40Trundlesetnosy: segfaulthunter, michael.foord, Trundle, daniel.urban
messages: + msg129024
2011-02-21 11:36:17michael.foordsetnosy: segfaulthunter, michael.foord, Trundle, daniel.urban
messages: + msg128958
2011-02-20 17:52:23Trundlesetfiles: + inspect_issue_11133.patch

nosy: + Trundle
messages: + msg128916

keywords: + patch
2011-02-20 12:48:24segfaulthuntersetnosy: + segfaulthunter
messages: + msg128904
2011-02-06 19:34:38michael.foordsetassignee: michael.foord
2011-02-06 19:34:21michael.foordsetmessages: + msg128073
2011-02-06 16:30:21daniel.urbancreate