[Patches] [ python-Patches-703471 ] (Security Problem) base64.decodestring exposes garbage value

[SourceForge.net]

Patches item #703471, was opened at 2003-03-14 09: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: Thomas Wouters (twouters)
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: Thomas Wouters (twouters)
Date: 2003-03-16 23: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 20: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 19: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 19: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 19: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 18: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 18: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 18: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 18: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 17: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 16: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