From a full disclosure email of Johnny Lee <infozip@gmail.com>: (and this is copied line by line, the proposed changes were not attached) No one can compile unzip with debug info? :) I looked at this on Windows. Unzip5.52 fixed a buffer overflow in the do_wild() function in all the ports. However, in vms\vms.c, there's still an unbounded strcpy call copying wld to last_wild. I don't know if there are any bounds to command line args in VMS, but if the first arg can be >256 chars, there's a buffer overflow there. In process.c, in process_zipfiles(), there's the following code: char *p = lastzipfn + strlen(lastzipfn); G.zipfn = lastzipfn; strcpy(p, ZSUFX); The fix for the do_wild() buffer overflow was to cap the string at FILNAMSIZ chars. So if we get to this point, then the strcpy of the ZSUFX will cause a slight buffer overflow of lastzipfn which typically points to G.matchname or a static matchname char array. It'd be really hard to exploit too, but I'm just mentioning it. The other possible buffer overflow is the Info macro in unzpriv.h. Code would end up using sprintf() to the slide array which is 32KB if MALLOC_WORK is NOT defined. I can't repro a segfault/access violation in the Win32 version of unzip 5.52 though unless I set the first argument to >32K via code. But you cannot create a Win32 process with a command line longer than 32K for WinXP+, (<= 2K on Win2K). With the Win32 DLL, it'd be possible for an app to pass in an arg that's >32K chars. Maybe on Unix with an arg that is a lot larger than 32K chars. Also a long environment variable may cause a buffer overflow. Updating the Info macro to use snprintf() would be the 'correct' thing to do. But all ports may not support snprintf() and the Info macro is used in many places. Adjusting all the uses of the Info macro to pass in the buffer size would take a long time. Time to whip out your favourite scripting language and write some code. You can use the C runtime (if it's ANSI-compliant) to avoid the Info macro's buffer overflow by passing a precision value for the string args. Here's a quick patch - I arbitrarily chose 512 as the max chars displayed: --- consts.old Sat Mar 23 10:52:48 2002 +++ consts.h Tue Jan 03 16:36:54 2006 @@ -34,9 +34,9 @@ "error: expected central file header signature not found (file #%lu).\n"; ZCONST char Far SeekMsg[] = "error [%s]: attempt to seek before beginning of zipfile\n%s"; -ZCONST char Far FilenameNotMatched[] = "caution: filename not matched: %s\n"; +ZCONST char Far FilenameNotMatched[] = "caution: filename not matched: %.512s\n"; ZCONST char Far ExclFilenameNotMatched[] = - "caution: excluded filename not matched: %s\n"; + "caution: excluded filename not matched: %.512s\n"; #ifdef VMS ZCONST char Far ReportMsg[] = "\ --- process.old Sun Nov 21 19:42:54 2004 +++ process.c Tue Jan 03 16:56:48 2006 @@ -74,20 +74,20 @@ /* do_seekable() strings */ # ifdef UNIX static ZCONST char Far CannotFindZipfileDirMsg[] = - "%s: cannot find zipfile directory in one of %s or\n\ - %s%s.zip, and cannot find %s, period.\n"; + "%s: cannot find zipfile directory in one of %.512s or\n\ + %s%.512s.zip, and cannot find %.512s, period.\n"; static ZCONST char Far CannotFindEitherZipfile[] = - "%s: cannot find or open %s, %s.zip or %s.\n"; + "%s: cannot find or open %.512s, %.512s.zip or %.512s.\n"; # else /* !UNIX */ # ifndef AMIGA static ZCONST char Far CannotFindWildcardMatch[] = - "%s: cannot find any matches for wildcard specification \"%s\".\n"; + "%s: cannot find any matches for wildcard specification \"%.512s\".\n"; # endif /* !AMIGA */ static ZCONST char Far CannotFindZipfileDirMsg[] = - "%s: cannot find zipfile directory in %s,\n\ - %sand cannot find %s, period.\n"; + "%s: cannot find zipfile directory in %.512s,\n\ + %sand cannot find %.512s, period.\n"; static ZCONST char Far CannotFindEitherZipfile[] = - "%s: cannot find either %s or %s.\n"; + "%s: cannot find either %.512s or %.512s.\n"; # endif /* ?UNIX */ extern ZCONST char Far Zipnfo[]; /* in unzip.c */ #ifndef WINDLL
The first one is VMS, the second one is unconfirmed on Unix. Setting this to Auditing for a confirmation.
Auditors any news on this one?
This looks like nonsense to me, I cant see how that would overflow, but there are a lot of Info() calls (~400), so lets say I've missed the ones he's talking about and a long command line or environment variable can do that, there is no impact as unzip is not suid/sgid, there is no suggestion that this could be triggered via a malformed archive, therefore I'm closing this as WORKSFORME.
