Tampering with signed cookies.

• Key: MODPYTHON-191
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Graham Dumpleton
• Reporter: Graham Dumpleton
• Component/s: core
• Affects Version/s: 3.2.10
• Fix Version/s: 3.3.1

Description

As reported by Andy Pearce in:

  http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/[e-mail]%3e

Andy Pearce wrote:
>
> Hi,
>
> I think I might have spotted a slight bug in Session.py. When the
> 'secret' parameter is supplied to use the SignedCookie class, it appears
> that __init__ of BaseSession doesn't check the return type of
> get_cookies().
>
> If I understand the SignedCookie docs correctly, if the cookie value
> doesn't match its signature, it simply returns the contents as a Cookie
> rather than a SignedCookie (indicating that the user tampered with their
> cookie before sending it back).
>
> However, there is no check in BaseSession's __init__ that the return of
> get_cookies() is a SignedCookie in the case that 'secret' is supplied.
>
> Perhaps a minor point, but it would seem to make the option of using
> SignedCookies rather pointless, since the signature isn't being checked.
> Presumably if the cookie has been tampered with, your only safe option
> is to throw it away and generate a new one. I think this can be achieved
> by changing the lines:
>
> if cookies.has_key(session_cookie_name):
> self._sid = cookies[session_cookie_name].value
>
> To something like:
>
> if cookies.has_key(session_cookie_name):
> if not secret or type(cookes[session_cookie_name]) \
> is Cookie.SignedCookie:
> self._sid = cookies[session_cookie_name].value
>
> I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> a quick explanation of why would be very much appreciated! ^_^
>
> Thanks,
>
> - Andy
>

Is this correct and should the change suggested appropriate?