[ python-Bugs-1523610 ] PyArg_ParseTupleAndKeywords potential core dump

[SourceForge.net]

Bugs item #1523610, was opened at 2006-07-16 19:23
Message generated for change (Comment added) made by ehuss
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1523610&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Interpreter Core
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Eric Huss (ehuss)
Assigned to: Nobody/Anonymous (nobody)
Summary: PyArg_ParseTupleAndKeywords potential core dump

Initial Comment:
After getting bit by bug 893549, I was noticing that
sometimes I was getting a core dump instead of a
TypeError when PyArg_ParseTupleAndKeywords was skipping
over a type the "skipitem" code does not understand.

There are about 4 problems I will describe (though they
are related, so it's probably not worth filing seperate
bugs).

The problem is that the "levels" variable is passed to
the seterror function uninitialized.  If levels does
not happen to contain any 0 elements, then the
iteration code in seterror will go crazy (I will get to
this in a moment).

In the one place where "skipitem" is called, you will
notice it immediately calls seterror() if it returned
an error message.  However, "levels" is not set by the
skipitem function, and thus seterror iterates over an
uninitialized value.  I suggest setting levels[0] = 0
somewhere in the beginning of the code, since the
expectations of setting the "levels" seems awefully
delicate.

(As a side note, there's no bounds checking on the
levels variable, either.  It seems unlikely that
someone will have 32 levels of nested variables, but I
think it points to a general problem with how the
variable is passed around.)

A second fix is to set levels[0] = 0 if setitem fails
before calling seterror().

Now, returning to the "seterror goes crazy" problem I
mentioned earlier, the following code in the seterror
function:

while (levels[i] > 0 && (int)(p-buf) < 220) {

should be:

while (levels[i] > 0 && (int)(p-buf) > 220) {

At least, that's what I'm assuming it is supposed to
be.  I think it should be clear why this is bad.

But wait, there's more!  The snprintf in seterror call
uses the incorrect size of buf.  The following line:

PyOS_snprintf(p, sizeof(buf) - (buf - p),

should be:

PyOS_snprintf(p, sizeof(buf) - (p - buf),

My particular platform (FreeBSD) puts a NUL character
at the end of the buffer.  However, since the size of
the buffer is computed incorrectly, this line of code
stomps on the stack (overwritting the levels value in
my case).

Let me know if you have any questions, or want any
sample code.


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

>Comment By: Eric Huss (ehuss)
Date: 2006-07-16 19:28

Message:
Logged In: YES 
user_id=393416

Oops, skip the section about <220 being >220.  I've been
staring at it too long.  The rest of the issues should be
valid, though.


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

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