[CC'ed to python-dev and Tim O'Malley] The Cookie module recently added to 2.0 provides 3 classes of Cookie: SimpleCookie, which treats cookie values as simple strings, SerialCookie, which treats cookie values as pickles and unpickles them, and SmartCookie which figures out if the value is a pickle or not. Unpickling untrusted data is unsafe. (Correct?) Therefore, SerialCookie and SmartCookie really shouldn't be used, and Moshe's docs for the module say so. Question: should SerialCookie and SmartCookie be removed? If they're not there, people won't accidentally use them because they didn't read the docs and missed the warning. Con: breaks backward compatibility with the existing cookie module and forks the code. (Are marshals safer than pickles? What if SerialCookie used marshal instead?) --amk
A.M. Kuchling writes:
(Are marshals safer than pickles? What if SerialCookie used marshal instead?)
A bit safer, I think, but this maintains the backward compatibility issue. If it is useful to change the API, this is the best time to do it, but we'd probably want to rename the module as well. Shared maintenance is also an issue -- Tim's opinion is very valuable here! -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
hola. On Wed, Aug 30, 2000 at 10:09:16AM -0400, Fred L. Drake, Jr. wrote:
A.M. Kuchling writes:
(Are marshals safer than pickles? What if SerialCookie used marshal instead?)
A bit safer, I think, but this maintains the backward compatibility issue.
Is this true? Marshal is backwards compatible to Pickle? If it is true, that'd be kinda cool.
If it is useful to change the API, this is the best time to do it, but we'd probably want to rename the module as well. Shared maintenance is also an issue -- Tim's opinion is very valuable here!
I agree -- if this is the right change, then now is the right time. If a significant change is warranted, then the name change is probably the right way to signal this change. I'd vote for 'httpcookie.py'. I've been thinking about the shared maintenance issue, too. The right thing is for the Cookie.py (or renamed version thereof) to be the official version. I would probably keep the latest version up on my web site but mark it as 'deprecated' once Python 2.0 gets released. thoughts..? e
On Wed, Aug 30, 2000 at 03:09:13PM -0400, timo@timo-tasi.org wrote:
hola. On Wed, Aug 30, 2000 at 10:09:16AM -0400, Fred L. Drake, Jr. wrote:
A.M. Kuchling writes:
(Are marshals safer than pickles? What if SerialCookie used marshal instead?)
A bit safer, I think, but this maintains the backward compatibility issue.
Is this true? Marshal is backwards compatible to Pickle?
No, what Fred meant is that it maintains the backward compatibility *issue*, not compatibility itself. It's still a problem for people who want to read cookies made by the 'old' version, or otherwise want to read in 'old' cookies. I think it would be possible to provide a 'safe' unpickle, that only unpickles primitives, for example, but that might *still* maintain the backwards compatibility issue, even if it's less of an issue then. And it's a bloody lot of work, too :-) -- Thomas Wouters <thomas@xs4all.net> Hi! I'm a .signature virus! copy me into your .signature file to help me spread!
On Wed, Aug 30, 2000 at 10:09:16AM -0400, Fred L. Drake, Jr. wrote:
A bit safer, I think, but this maintains the backward compatibility issue.
timo@timo-tasi.org writes:
Is this true? Marshal is backwards compatible to Pickle?
If it is true, that'd be kinda cool.
Would be, but my statement wasn't clear: it maintains the *issue*, not compatibility. ;( The data formats are not interchangable in any way. -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
"Fred L. Drake, Jr." <fdrake@beopen.com>:
it maintains the *issue*, not compatibility. ;(
A confusing choice of word! Usually one only talks about "maintaining" something that one *want* maintained... Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
hola. On Wed, Aug 30, 2000 at 09:26:20AM -0400, A.M. Kuchling wrote:
Question: should SerialCookie and SmartCookie be removed? If they're not there, people won't accidentally use them because they didn't read the docs and missed the warning.
Con: breaks backward compatibility with the existing cookie module and forks the code.
I had a thought about this - kind of a intermediate solution. Right now, the shortcut 'Cookie.Cookie()' returns an instance of the SmartCookie, which uses Pickle. Most extant examples of using the Cookie module use this shortcut. We could change 'Cookie.Cookie()' to return an instance of SimpleCookie, which does not use Pickle. Unfortunately, this may break existing code (like Mailman), but there is a lot of code out there that it won't break. Also, people could still use the SmartCookie and SerialCookie classes, but not they would be more likely to read them in the documentation because they are "outside the beaten path".
"timo" == <timo@timo-tasi.org> writes:
timo> Right now, the shortcut 'Cookie.Cookie()' returns an timo> instance of the SmartCookie, which uses Pickle. Most extant timo> examples of using the Cookie module use this shortcut. timo> We could change 'Cookie.Cookie()' to return an instance of timo> SimpleCookie, which does not use Pickle. Unfortunately, timo> this may break existing code (like Mailman), but there is a timo> lot of code out there that it won't break. Not any more! Around the Mailman 2.0beta5 time frame, I completely revamped Mailman's cookie stuff because lots of people were having problems. One of the things I suspected was that the binary data in cookies was giving some browsers headaches. So I took great pains to make sure that Mailman only passed in carefully crafted string data, avoiding Cookie.py's pickle stuff. I use marshal in the application code, and I go further to `hexlify' the marshaled data (see binascii.hexlify() in Python 2.0). That way, I'm further guaranteed that the cookie data will consist only of characters in the set [0-9A-F], and I don't need to quote the data (which was another source of browser incompatibility). I don't think I've seen any cookie problems reported from people using Mailman 2.0b5. [Side note: I also changed Mailman to use session cookies by default, but that probably had no effect on the problems.] [Side side note: I also had to patch Morsel.OutputString() in my copy of Cookie.py because there was a test for falseness that should have been a test for the empty string explicitly. Otherwise this fails: c['foo']['max-age'] = 0 but this succeeds c['foo']['max-age'] = "0" Don't know if that's relevant for Tim's current version.] timo> Also, people could still use the SmartCookie and timo> SerialCookie classes, but not they would be more likely to timo> read them in the documentation because they are "outside the timo> beaten path". My vote would be to get rid of SmartCookie and SerialCookie and stay with simple string cookie data only. Applications can do fancier stuff on their own if they want. -Barry
"AMK" == A M Kuchling <amk1@erols.com> writes:
AMK> (Are marshals safer than pickles? What if SerialCookie used AMK> marshal instead?) I would guess that pickle makes attacks easier: It has more features, e.g. creating instances of arbitrary classes (provided that the attacker knows what classes are available). But neither marshal nor pickle is safe. It is possible to cause a core dump by passing marshal invalid data. It may also be possible to launch a stack overflow attack -- not sure. Jeremy
On Wed, Aug 30, 2000 at 09:21:23PM -0400, Jeremy Hylton wrote:
... But neither marshal nor pickle is safe. It is possible to cause a core dump by passing marshal invalid data. It may also be possible to launch a stack overflow attack -- not sure.
I believe those core dumps were fixed. Seems like I remember somebody doing some work on that. ?? Cheers, -g -- Greg Stein, http://www.lyra.org/
"GS" == Greg Stein <gstein@lyra.org> writes:
GS> On Wed, Aug 30, 2000 at 09:21:23PM -0400, Jeremy Hylton wrote:
... But neither marshal nor pickle is safe. It is possible to cause a core dump by passing marshal invalid data. It may also be possible to launch a stack overflow attack -- not sure.
GS> I believe those core dumps were fixed. Seems like I remember GS> somebody doing some work on that. GS> ?? Aha! I hadn't notice that patch sneaking in. I brought it up with Guido a few months ago and he didn't want to make changes to marshal because, IIRC, marshal exists only because .pyc files need it. Jeremy
On Wed, Aug 30, 2000 at 06:53:10PM -0700, Greg Stein wrote:
On Wed, Aug 30, 2000 at 09:21:23PM -0400, Jeremy Hylton wrote:
... But neither marshal nor pickle is safe. It is possible to cause a core dump by passing marshal invalid data. It may also be possible to launch a stack overflow attack -- not sure.
I believe those core dumps were fixed. Seems like I remember somebody doing some work on that.
??
Nope, I think that there may have been a few small patches but the discussions to fix some "brokeness" in marshal did not bear fruit: http://www.python.org/pipermail/python-dev/2000-June/011132.html Oh, I take that back. Here is patch that supposedly fixed some core dumping: http://www.python.org/pipermail/python-checkins/2000-June/005997.html http://www.python.org/pipermail/python-checkins/2000-June/006029.html Trent -- Trent Mick TrentM@ActiveState.com
jeremy wrote:
I would guess that pickle makes attacks easier: It has more features, e.g. creating instances of arbitrary classes (provided that the attacker knows what classes are available).
well, if not else, he's got the whole standard library to play with... ::: (I haven't looked at the cookie code, so I don't really know what I'm talking about here) cannot you force the user to pass in a list of valid classes to the cookie constructor, and use a subclass of pickle.Unpickler to get a little more control over what's imported: class myUnpickler(Unpicker): def __init__(self, data, classes): self.__classes = classes Unpickler.__init__(self, StringIO.StringIO(data)) def find_class(self, module, name): for cls in self.__classes__: if cls.__module__ == module and cls.__name__ == name: return cls raise SystemError, "failed to import class"
But neither marshal nor pickle is safe. It is possible to cause a core dump by passing marshal invalid data. It may also be possible to launch a stack overflow attack -- not sure.
</F>
On Wed, Aug 30, 2000 at 09:21:23PM -0400, Jeremy Hylton wrote:
I would guess that pickle makes attacks easier: It has more features, e.g. creating instances of arbitrary classes (provided that the attacker knows what classes are available).
marshal can handle code objects. That seems pretty scary to me. I would vote for not including these unsecure classes in the standard distribution. Software that expects them should include their own version of Cookie.py or be fixed. Neil
"NS" == Neil Schemenauer <nascheme@enme.ucalgary.ca> writes:
NS> On Wed, Aug 30, 2000 at 09:21:23PM -0400, Jeremy Hylton wrote:
I would guess that pickle makes attacks easier: It has more features, e.g. creating instances of arbitrary classes (provided that the attacker knows what classes are available).
NS> marshal can handle code objects. That seems pretty scary to me. NS> I would vote for not including these unsecure classes in the NS> standard distribution. Software that expects them should NS> include their own version of Cookie.py or be fixed. If a server is going to use cookies that contain marshal or pickle data, they ought to be encrypted or protected by a secure hash. Jeremy
参加者 (11)
-
A.M. Kuchling -
bwarsaw＠beopen.com -
Fred L. Drake, Jr. -
Fredrik Lundh -
Greg Ewing -
Greg Stein -
Jeremy Hylton -
Neil Schemenauer -
Thomas Wouters -
timo＠timo-tasi.org -
Trent Mick