[Patches] [ python-Patches-703471 ] (Security Problem) base64.decodestring exposes garbage value
SourceForge.net
noreply@sourceforge.net
Sun, 16 Mar 2003 14:49:35 -0800
Patches item #703471, was opened at 2003-03-14 03:18
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=703471&group_id=5470
Category: Library (Lib)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 8
Submitted By: Hye-Shik Chang (perky)
>Assigned to: Barry A. Warsaw (bwarsaw)
Summary: (Security Problem) base64.decodestring exposes garbage value
Initial Comment:
>>> import base64
>>> base64.decodestring("###################")
'\x0cD\x1a\x08\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
>>> base64.decodestring(".....")
'ps2\x00\x00t'
>>> base64.decodestring("........................")
'\x0cF\x1a\x08\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
>>>
base64.decodestring(".................................................")
'.............................."\x00\x00\x00\x00\x00\x00\x00\x00'
This exposes unexpected values that deallocated recently.
(some my cgi script showed garbage that contains a
database password in offensive query)
----------------------------------------------------------------------
>Comment By: Tim Peters (tim_one)
Date: 2003-03-16 17:49
Message:
Logged In: YES
user_id=31435
The bug here is exposing recycled memory, so let's fix that
first. Changing under which conditions the function raises
exceptions is a different can of worms, and is probably off
the table for 2.2 backporting regardless. Barry, if you're
happy with the way the patch fixes the reported bug,
please accept it and assign it back to Thomas; if you want
to go on to change error-raising behavior for 2.3, better to
open another report.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-16 17:24
Message:
Logged In: YES
user_id=34209
Well, the patch restores the behaviour of Python 2.1 and
earlier (at least as far back as 1.5.2.) Also, binascii
ignores any errors *except* padding errors, and 'padding
errors' mean 'valid base64-characters left over'. Invalid
characters inside the base64 stream are silently ignored,
and in fact the base64 test explicitly tests this
behaviour... I think ignoring anything but whitespace in the
first place is the problem here, but that's not the problem
the patch tries to solve, and I don't know enough about the
official specs to say whether this is mandatory or not.
Added the changes Tim wanted to the patch.
----------------------------------------------------------------------
Comment By: Barry A. Warsaw (bwarsaw)
Date: 2003-03-14 14:25
Message:
Logged In: YES
user_id=12800
why wouldn't calling it on garbage data raise
binascii.Error? i think i'd feel more comfortable about the
patch if it did that instead (to be consistent with
incomplete padding errors and such).
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2003-03-14 13:19
Message:
Logged In: YES
user_id=31435
I'd like it fine if
1. It did rv = PyString_FromString("") and then fell thru to the
existing "return rv;", instead of creating another return point.
2. Add a comment about why this convolution is needed: this
part of the function has been implicating in two bugs so far.
The base64 stuff silently skips over garbage characters
because everything would break now if it didn't <wink>.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-14 13:17
Message:
Logged In: YES
user_id=34209
Assigning to Barry for review, on the 'last urinated'
principle <wink>.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-14 13:05
Message:
Logged In: YES
user_id=34209
Version of the patch with a test attached. It looks sane to
me, and it seems to work. I'm not sure why binascii isn't
raising an exception when receiving invalid data, but this
is python2.1-and-earlier behaviour, and I'm not about to
break that.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-14 12:23
Message:
Logged In: YES
user_id=34209
Hm, I see. I figured it was PyString_FromStringAndSize()'s
fault for not honoring the NULL source-string in the case of
a zero-length request, but I see how that might be intended.
How about this patch instead ?
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2003-03-14 12:14
Message:
Logged In: YES
user_id=31435
The thing I'm worried about is that _PyString_Resize must
not be called on a string that's empty to begin with (resizing
will fail because the empty string is shared, and the resize
routine checks for that).
The *last* patch to this function inserted the bin_len > 0 test
for what appears to be that very reason -- but that also
created the problem we're seeing now.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-14 12:07
Message:
Logged In: YES
user_id=34209
Ah, it is not. I'll see about fixing it (and writing the
testcase etc etc yahdah yahdah.)
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2003-03-14 12:05
Message:
Logged In: YES
user_id=31435
I raised the priority so someone would look at it. That part
worked <wink>. I'm unsure about the patch, but don't have
time to explain that now.
----------------------------------------------------------------------
Comment By: Thomas Wouters (twouters)
Date: 2003-03-14 11:48
Message:
Logged In: YES
user_id=34209
The patch seems to me to be the correct fix. Did you have a
reason to raise the priority but not check it in, Tim ?
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2003-03-14 10:19
Message:
Logged In: YES
user_id=31435
Yikes! Boosted priority way up. A quick check shows that
my Python 2.2.2 also appears to "decode" free'd RAM here
on Windows.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=703471&group_id=5470