zoo contains exploitable buffer overflows

[Josh Bressers]

> 
> As the FE zoo maintainer I've applied the security patch suggested on=20
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=3D183109
> 
> I'm not sure the security analysis here is right, but since the patch
> seems harmless and zoo is exposed to external input via mail filters
> such as amavisd-new I preferred to err on the side of caution.

The issue seems to exist.  You can get cause zoo to segfault upon archive
creation (this is a new and different issue) by creating a very long
directory path.

mkdir `perl -e 'print "A"x254'`
cd `perl -e 'print "A"x254'`
mkdir `perl -e 'print "A"x254'`
cd `perl -e 'print "A"x254'`
touch feh
cd ../..
zoo a arch.zoo `perl -e 'print "A"x254 . "/" . "A"x254 . "/feh"'`

This causes zoo to crash here:

    void parse (path_st, fname)
    register struct path_st *path_st;
    char *fname;
    {
       char tempname[LFNAMESIZE];       /* working copy of supplied fname */
       char *namep;                   /* points to relevant part of tempname */

       char *p;
       strcpy (tempname, fname); <== Buffer overflow

This points out that zoo is a very poorly written program.  Luckily with
the new changes to gcc and glibc, these horrible stack buffer overflows are
non issues.  Run the above commands, you'll see on FC5 it doesn't crash,
libc catches it.


> If some people could review the alert and the patch I'd be grateful.
> To my knowledge other distributions have not acted on the alert yet
> (it's been published on many security lists in the last days).

The patch attached to the mail (in bugzilla) looks pretty hokey.  I would
either malloc however much space is needed, or verify the path won't
overflow the static space.  Adding more space to a static buffer doesn't
help, it should still be possible to overflow the buffer.  The only good
way to fix this I can see is modify the combine() function to either accept
a maximum string length, or malloc the space itself.

If you pass a length to combine(), you have the issue of uncleanly exiting.
The code I see has no nice way to report potential errors, which means
you'll have to exit() inside combine(), which will leave things in an
unknown state.

Fixing zoo is probably never going to happen, this is just one of the
things that is horribly broken by design.  From my quick look through the
source, it's pretty bad.  There are going to be countless similar problems

-- 
    JB