pickle should support methods

[exarkun]

BPO	9276
Nosy	@malemburg, @loewis, @rhettinger, @amauryfa, @abalkin, @pitrou, @avassalotti, @merwok, @durban, @cool-RR
Dependencies	bpo-17810: Implement PEP 3154 (pickle protocol 4)

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-11-30.03:51:13.854>
created_at = <Date 2010-07-16.19:25:18.662>
labels = ['type-feature', 'library']
title = 'pickle should support methods'
updated_at = <Date 2013-11-30.03:51:13.851>
user = 'https://bugs.python.org/exarkun'

bugs.python.org fields:

activity = <Date 2013-11-30.03:51:13.851>
actor = 'alexandre.vassalotti'
assignee = 'none'
closed = True
closed_date = <Date 2013-11-30.03:51:13.854>
closer = 'alexandre.vassalotti'
components = ['Library (Lib)']
creation = <Date 2010-07-16.19:25:18.662>
creator = 'exarkun'
dependencies = ['17810']
files = []
hgrepos = []
issue_num = 9276
keywords = []
message_count = 39.0
messages = ['110473', '112405', '112428', '112446', '112450', '112453', '112462', '112467', '112468', '112471', '112473', '112474', '112475', '112476', '112477', '112479', '112482', '112483', '112486', '112488', '112496', '129202', '129790', '129797', '129798', '129801', '129803', '129804', '129805', '129806', '129809', '129812', '129813', '129867', '129868', '129882', '130054', '148771', '204770']
nosy_count = 13.0
nosy_names = ['lemburg', 'loewis', 'rhettinger', 'hinsen', 'exarkun', 'amaury.forgeotdarc', 'belopolsky', 'pitrou', 'alexandre.vassalotti', 'eric.araujo', 'obamausa8', 'daniel.urban', 'cool-RR']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue9276'
versions = ['Python 3.4']

[exarkun]

pickle doesn't support methods:

>>> class x:
...     def y(self):
...             pass
... 
>>> import pickle
>>> pickle.dumps(x.y)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/exarkun/Projects/python/branches/py3k/Lib/pickle.py", line 1314, in dumps
    Pickler(f, protocol, fix_imports=fix_imports).dump(obj)
_pickle.PicklingError: Can't pickle <class 'function'>: attribute lookup builtins.function failed

It would be easy to fix this, though. Here's a link to some code that implements it: http://twistedmatrix.com/trac/browser/trunk/twisted/persisted/styles.py?rev=1

[abalkin]

Not a proposed solution, but food for thought. Methods do have __reduce_ex__ method which works with protocol 3:

>>> class X:
...   def f(self):
...      pass

>>> X.f.__reduce_ex__(3)
(<function __newobj__ at 0x100579288>, (<class 'function'>,), {}, None, None)

This result is useless for several reasons:

1. <class 'function'> cannot be pickled because although it's name suggests a builtin, it is only available as types.FunctionType.

2. If we define builtins.function, X.f can be pickled

>>> import builtins, types
>>> builtins.function = types.FunctionType
>>> pickle.dumps(X.f)
b'\x80\x03cbuiltins\nfunction\nq\x00)\x81q\x01}q\x02b.'

but the result is useless:

>>> pickle.loads(_)
Traceback (most recent call last):
 ..
 File "Lib/pickle.py", line 1317, in loads
    encoding=encoding, errors=errors).load()
TypeError: Required argument 'code' (pos 1) not found

I think the approach of pickling the name of the function as is done in the Twisted link above is the only reasonable one and is consistent with the way module level functions are pickled.

[malemburg]

Note that pickle deliberately does not support serializing code objects.

This is a security feature and should not be broken !

If you need to pickle such objects, you can easily register handlers that take care of this.

[exarkun]

This is a security feature and should not be broken !

Can you explain this?

I don't think I agree, since an attacker can always serialize whatever they feel like. It's the person doing the deserialization that has to be careful.

[malemburg]

Jean-Paul Calderone wrote:

Jean-Paul Calderone <exarkun@twistedmatrix.com> added the comment:

> This is a security feature and should not be broken !

Can you explain this?

I don't think I agree, since an attacker can always serialize whatever they feel like. It's the person doing the deserialization that has to be careful.

The marshal protocol which is used for storing PYC files has support
for serializing code objects.

The support on pickles, which are meant for data serialization, was not added
per default to prevent unwanted code execution during deserialization,
but instead made possible via pickle hooks, so as to make the decision
to support code serialization an explicit application choice.

By adding default support for unpickling code objects, you can trick
the unpickling code into executing serialized code: first you add
a serialized version of a malicious class definition, then you add
an object of that class to the pickle. At object restore time, the
malicious class can then run os.system('rm -rf /')...

[exarkun]

By adding default support for unpickling code objects, you can trick
the unpickling code into executing serialized code:

This doesn't sound correct to me.

You can *already* trick unpickling code into executing serialized code. You don't need this feature in order to be able to do it.

[malemburg]

Jean-Paul Calderone wrote:

Jean-Paul Calderone <exarkun@twistedmatrix.com> added the comment:

> By adding default support for unpickling code objects, you can trick
the unpickling code into executing serialized code:

This doesn't sound correct to me.

You can *already* trick unpickling code into executing serialized code. You don't need this feature in order to be able to do it.

How ?

[exarkun]

For example:

exarkun@boson:~$ python
Python 2.6.4 (r264:75706, Dec  7 2009, 18:45:15) 
[GCC 4.4.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class x(object):
...     def __reduce__(self):
...         import os
...         return os.system, ('echo "Hello from sploitland"',)
... 
>>> import pickle
>>> pickle.loads(pickle.dumps(x()))
Hello from sploitland
0
>>>

[abalkin]

On Mon, Aug 2, 2010 at 9:25 AM, Marc-Andre Lemburg
<report@bugs.python.org> wrote:
..
>> You can *already* trick unpickling code into executing serialized code.  You don't need
> this feature in order to be able to do it.
>
> How ?
>
>>> from pickle import *
>>> class evil:
...    def __reduce__(self):
...        return (exec, ("print('pwned!')",))
...
>>> s = dumps(evil())
>>> loads(s)
pwned!

See also http://bugs.python.org/issue9120#msg109004 .

AFAICT, the reason functions and classes are pickled by name has
nothing to do with security. From the manual:

"""Similarly, when class instances are pickled, their class’s code and
data are not pickled along with them. Only the instance data are
pickled. This is done on purpose, so you can fix bugs in a class or
add methods to the class and still load objects that were created with
an earlier version of the class. If you plan to have long-lived
objects that will see many versions of a class, it may be worthwhile
to put a version number in the objects so that suitable conversions
can be made by the class’s __setstate__() method.
""" http://docs.python.org/library/pickle.html?#what-can-be-pickled-and-unpickled

[malemburg]

Jean-Paul Calderone wrote:
> 
> Jean-Paul Calderone <exarkun@twistedmatrix.com> added the comment:
> 
> For example:
> 
> exarkun@boson:~$ python
> Python 2.6.4 (r264:75706, Dec  7 2009, 18:45:15) 
> [GCC 4.4.1] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
>>>> class x(object):
> ...     def __reduce__(self):
> ...         import os
> ...         return os.system, ('echo "Hello from sploitland"',)
> ... 
>>>> import pickle
>>>> pickle.loads(pickle.dumps(x()))
> Hello from sploitland
> 0

But here you are not transferring malicious code in the pickle
string, you are just triggering the execution of such code that
you already have (and are in control of).

Without the definition of class x on the receiving side, there
would be no exploit.

By adding support for pickling code objects, you'd make it possible
to place the definition of class x into the pickle string and
you would no longer be in control of that code.

[abalkin]

On Mon, Aug 2, 2010 at 10:05 AM, Marc-Andre Lemburg
<report@bugs.python.org> wrote:
..

Without the definition of class x on the receiving side, there
would be no exploit.

You are mistaken. Try adding del x (or del evil in my example)
between dumps and loads and see it working.

[pitrou]

I think methods should be picklable just like global functions are, that is, by pickling a tuple of the fully-qualified class name ("mymodule.myclass"), method name ("mymethod"), and self.

[malemburg]

M.-A. Lemburg wrote:
> Jean-Paul Calderone wrote:
>>
>> Jean-Paul Calderone <exarkun@twistedmatrix.com> added the comment:
>>
>> For example:
>>
>> exarkun@boson:~$ python
>> Python 2.6.4 (r264:75706, Dec  7 2009, 18:45:15) 
>> [GCC 4.4.1] on linux2
>> Type "help", "copyright", "credits" or "license" for more information.
>>>>> class x(object):
>> ...     def __reduce__(self):
>> ...         import os
>> ...         return os.system, ('echo "Hello from sploitland"',)
>> ... 
>>>>> import pickle
>>>>> pickle.loads(pickle.dumps(x()))
>> Hello from sploitland
>> 0
> 
> But here you are not transferring malicious code in the pickle
> string, you are just triggering the execution of such code that
> you already have (and are in control of).
> 
> Without the definition of class x on the receiving side, there
> would be no exploit.
> 
> By adding support for pickling code objects, you'd make it possible
> to place the definition of class x into the pickle string and
> you would no longer be in control of that code.

Hmm, I just tried the code and it seems that you're right:

The pickle string does not contain a reference to class x,
but only the name of the function to call. Wow, that's a huge
hole in Python's pickle system...

...  def __reduce__(self):
...   import os
...   return os.system, ('echo "Bingo"',)
...
>>> import pickle
>>> pickle.dumps(C())
'cposix\nsystem\np0\n(S\'echo "Bingo"\'\np1\ntp2\nRp3\n.'
>>> C = None
>>> s = 'cposix\nsystem\np0\n(S\'echo "Bingo"\'\np1\ntp2\nRp3\n.'
>>> pickle.loads(s)
Bingo
0