[Python-bugs-list] [ python-Bugs-471893 ] Security review of pickle/marshal docs

[noreply@sourceforge.net]

Bugs item #471893, was opened at 2001-10-16 15:42
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=471893&group_id=5470

Category: Documentation
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Tim Peters (tim_one)
Assigned to: Jeremy Hylton (jhylton)
Summary: Security review of pickle/marshal docs

Initial Comment:
Paul Rubin points out that the security implications 
of using marshal and/or pickle aren't clear from the 
docs.  Assigning to Jeremy as he's more sensitive to 
such issues than I am; maybe Barry would like to get 
paranoid too <wink>.

A specific example:  the pickle docs say that pickle 
doesn't support code objects, and "at least this 
avoids the possibility of smuggling Trojan horses into 
a program".  However,

1) The marshal docs don't mention this vulnerability 
at all.

while

2) The pickle docs don't spell out possible dangers 
due to things pickle does that marshal doesn't (like 
importing modules, and running class constructors).

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

Comment By: Nobody/Anonymous (nobody)
Date: 2001-11-07 12:08

Message:
Logged In: NO 

Irmen de Jong points out that the standard cookie module
uses pickling for serial and smart cookies.  The 2.1.1
cookie module docs explicitly say not to use those
classes because of the security hole--that they're provided
for backward compatibility only (but with what?!  Any
server that uses those classes on open ports needs to be
changed right away).

Irmen's library, http://pyro.sourceforge.net, also uses
unpickle insecurely (he's aware of this now and is figuring
out a fix).

IMO this is really a code bug rather than a documentation
bug, and should be fixed in the code rather than the docs.
Documenting the bug rather than fixing it leaves a
deficiency in the Python library: obvious uses of pickling,
like Pyro and the cookie module, can't be implemented
using cPickle and have to resort to a slower Python
deserializer, or use marshal and have possible compatibility
problems between versions (and not be able to serialize
class instances).

Paul



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

Comment By: paul rubin (phr)
Date: 2001-10-16 16:06

Message:
Logged In: YES 
user_id=72053

Certainly anyone unserializing potentially malicious data
with pickle, marshal, or anything else, should check the
results before doing anything dangerous with them (like
executing code).  However, unpickle can potentially do
damage before it even returns, by creating loading modules
and creating initialized class instances.  So pickle.loads
should never be used on untrusted strings, except possibly
in a rexec wrapper (as proposed by Tim).  Another
possibility (also by Tim) is to override the load_inst
method of the Pickler class, though I don't think you can
do that for cPickle.

A sample exploit for unpickle can be found at
<http://www.nightsong.com/phr/python/pickletest.py>.
Unpickling the test string runs penguin.__init__ contrary
to the doc's saying no initialization unless there's a
__getinitargs__ method in the class def.

The "exploding penguin" class is artificial, but
applications are vulnerable if there's an unsafe
constructor anywhere in any class of the application or in
the python library (example: the NNTP constructor opens an
IP connection to an arbitrary address, so a malicious
imported string can send a message through your firewall
when you import it).


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

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