| msg82272 - (view) |
Author: James William Pye (jwp) |
Date: 2009-02-16 21:23 |
| I found this bug by misplacing a line of a code. Yes, I was doing naughty things, but in the case of the class that led to the discovery, it was inadvertent. :P class something_else(object): pass class foo(object): def __del__(self): self.__class__ = something_else for _ in range(1000): foo() That results in a fatal python error due to a negative reference count(on the foo class object) in 3.0. 3.0.1 (release30-maint:69593M, Feb 13 2009, 14:48:10) -> kaboom jwp@torch[]:org/pgfoundry/python 134% /src/build/py30/bin/python3.0 ./kaboom.py Fatal Python error: Objects/descrobject.c:10 object at 0x5221b8 has negative ref count -1 zsh: abort /src/build/py30/bin/python3.0 ./kaboom.py 2.6 (r26:66714, Dec 21 2008, 21:17:32) -> kaboom jwp@torch[]:org/pgfoundry/python 0% /sw/bin/python2.6 ./kaboom.py Fatal Python error: GC object already tracked zsh: abort /sw/bin/python2.6 ./kaboom.py 2.5.2 (r252:60911, Jun 15 2008, 18:55:39) -> no kaboom (no asserts? eh..) ... 2.5.1 (r251:54863, Apr 15 2008, 22:57:26) -> kaboom (/usr/bin/python2.5) jwp@torch[]:org/pgfoundry/python 0% /usr/bin/python2.5 ./kaboom.py Assertion failed: (PyType_Check(base)), function _PyType_Lookup, file Objects/typeobject.c, line 2035. zsh: abort /usr/bin/python2.5 ./kaboom.py |
|
|
| msg82354 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *  |
Date: 2009-02-17 19:04 |
| Yes, the wrong type is DECREF'd when the object is deallocated. Patch attached. |
|
|
| msg82427 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-02-18 19:55 |
| The patch looks correct, but are there more places where the type is kept in a local variable while Python code is invoked? |
|
|
| msg82439 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2009-02-18 22:51 |
| I carefully looked at all places that store ->ob_type or Py_TYPE() in a local variable, and I could not find any exploit. Most places don't reuse the type once the method or the slot has been called. Two places were harder to analyze: subtype_clear (but an attack would use __del__, and use a reference cycle: subtype_clear is never called in this case) and PyObject_Generic(Get|Set)Attr (the only escape path to python code could be through PyType_Ready; but it has already been called for heap types) |
|
|
| msg82446 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-02-19 01:37 |
| On Wed, Feb 18, 2009 at 4:51 PM, Amaury Forgeot d'Arc <report@bugs.python.org> wrote: > > Amaury Forgeot d'Arc <amauryfa@gmail.com> added the comment: > > I carefully looked at all places that store ->ob_type or Py_TYPE() in a > local variable, and I could not find any exploit. Most places don't > reuse the type once the method or the slot has been called. Thanks for looking! > > Two places were harder to analyze: subtype_clear (but an attack would > use __del__, and use a reference cycle: subtype_clear is never called in > this case) and PyObject_Generic(Get|Set)Attr (the only escape path to > python code could be through PyType_Ready; but it has already been > called for heap types) Well, I think we can deal with those if they are reported. Go ahead and apply the patch. |
|
|
| msg82719 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2009-02-25 21:15 |
| Could the PyObject_ClearWeakRefs(self); call in the middle of the lines del_changes_class.patch adds also be used to cause python code to set __del__ or __dict__ causing the wrong destructor or wrong dict to be DECREFed? (I'm trying to wrap my head around the possibilities here and come up with a test case of that but weakref isn't behaving as i'd expect) |
|
|
| msg86442 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-04-25 00:45 |
| Fixed in r71860. |
|
|