Re: first patch to security problem

[Peter <zed@linuxpower.org>]

On Sat, 26 Aug 2000, Decklin Foster wrote:

> Mike Guidero writes:
> 
> > That sounded like what was happening as I was trying eliminate the
> > IRC::user_list() bug in perl.c  once I overflowed the buffer, it
> > would crash in unrelated areas...
> > 
> > Hrmm... but I fixed that one :)
> 
> I suppose there are probably others lurking, all it takes is
> overwriting something that 'belongs' to us and thus doesn't produce a
> segfault. OTOH, i could very easily be to blame. If you want to try
> sifting through the rest of the code, here's a chunk-allocation
> version which (for me) segfaults upon accessing elment 5 if the buffer
> is reallocd from 10 to 20. All my sanity-checking gunk is still in
> there.
> 
> However, while reading backtraces, I noticed that xchat has the
> command stored split into words, and then converts that back into a
> single string where the my_system_* stuff gets it. I would be much
> better if we could just make all this code irrelevant by not
> un-parsing the data, and pass the array of words directly.

I wouldn't think so. It just grabs the relevent URL handler ("netscape
%s" or whatever) from the urlhandler_list and sticks in %s using
autoinsert(). I don't think it's ever split into words before that.

#define CHUNKSIZE 10

void
my_system_noshell (char *cmd)
{
    char *token = NULL, **argv = g_malloc(CHUNKSIZE * sizeof *argv), **tmp;

I think that line looks dodgy. sizeof *argv, isn't that derefencing an
uninitialize, unalloced variable? Maybe I'm wrong, I've never used that
syntax before. I would try something like:

   **argv = g_malloc(CHUNKSIZE * sizeof (char *));

and another thing, shouldn't your realloc() take into account the size of
a pointer?

   tmp = g_realloc(argv, (++chunks) * CHUNKSIZE);

   ....

    //free(argv); /* FIXME: crash */

A g_malloc needs a g_free :)

Anyway, I've taken the poptParseArgs function from gnome-libs 1.2.4 and it
seems to work well. It also handles quotoes correctly (so you can have
args with a space in them). I've #ifdef USE_GNOME'ed it, so it's only
included when not building with gnome.

-- 
Peter. <zed@linuxpower.org>

-
XChat-discuss: mailing list for XChat users
Archive:       http://mail.nl.linux.org/lists/
Posted By:     Peter <zed@linuxpower.org>