Fw: Re: buffer overrun in zlib 1.1.4

[Niki Guldbrand]

Begin forwarded message:

Date: Mon, 24 Feb 2003 07:25:11 -0500 (PET)
From: Carlo Marcelo Arenas Belon <carenas at chasqui.lared.net.pe>
To: Richard Kettlewell <rjk at greenend.org.uk>
Subject: Re: buffer overrun in zlib 1.1.4


> There is an internal #define (HAS_vsnprintf) that causes it to use
> vsnprintf() instead of vsprintf(), but this is not enabled by default,
> not tested for by the configure script, and not documented.

the configure script on zlib is not generated by autoconf and is optional
when building; therefore there is no config.h, and the included file
"zconf.h" that is the one used for system related configuration is static.

something interesting though, is that the preprocessor variables being
tested are HAVE_* instead of HAS_* (HAS_vsnprintf and HAS_snprintf), as 
the ones found on gzio.c, what could help to explain why it is not 
documented, neither tested for.

from the Changelog it seems those functions were added on version 1.0.6
(Jan 19, 1998) by Roland Giersig and Kevin Ruland, and probably they never 
included the test on configure for that.

> Even if it was documented, tested for, or whatever, it is unclear what
> platforms without vsnprintf() are supposed to do.  Put up with the
> security hole, perhaps.

from the code it seems that they are supposed to use vsprintf (on an 
ANSI C environment) or sprintf (if not ANSI C).

on any case, long strings will be silently truncated and overflows are 
possible as the one you coded

> Finally, with HAS_vsnprintf defined, long strings will be silently
> truncated (and this isn't documented anywhere).  Unexpected truncation
> of strings can have security implications too; I seem to recall that a
> popular MTA had trouble with over-long HELO strings for instance.

the attached patch fixes both of the problems, even if it breaks on 
systems with a broken [v]snprintf (any one yet?) and that could be 
considered a prerequisite for building, probably using a custom made 
[v]snprintf implementation like the one on :

  http://www.ijs.si/software/snprintf/

regards,

Carlo



-- 
Med Venlig Hilsen / Best Regards
                              |  Teleservice Esbjerg A/S
Niki Guldbrand                |  Salingsundvej 4
IT-Administrator              |  6715 Esbjerg N
                              |  Denmark
Phone         : +45 79144544  |
Direct Phone  : +45 79144589  |  Web : http://www.teleservice.com
Fax           : +45 79144599  |

E-Mail        : Niki.Guldbrand at teleservice.com

--------------
Those who hate and fight must stop themselves -- otherwise it is not stopped.
		-- Spock, "Day of the Dove", stardate unknown
--------------
-------------- next part --------------
diff -ur zlib-1.1.4/configure zlib-1.1.4-vsnprintf/configure
--- zlib-1.1.4/configure	Wed Jul  8 13:19:35 1998
+++ zlib-1.1.4-vsnprintf/configure	Mon Feb 24 00:06:55 2003
@@ -167,6 +167,54 @@
 fi
 
 cat > $test.c <<EOF
+#include <stdio.h>
+#include "zconf.h"
+
+#ifdef STDC
+
+#include <stdarg.h> 
+
+int test (const char *format, /* args */ ...)
+{
+  char buf[10];
+  va_list va;
+  int len;
+
+  va_start(va, format);
+  len = vsnprintf(buf, sizeof(buf), format, va);
+  va_end(va);
+
+  return len;
+}
+
+#else /* not ANSI C */
+
+int test(format, a1, a2)
+  const char *format;
+  int a1, a2;
+{
+  char buf[10];
+  int len;
+
+  len = snprintf(buf, sizeof(buf), format, a1, a2);
+
+  return len;
+}
+#endif
+
+int main(void)
+{
+  exit(0);
+}
+EOF
+if test "`($CC -c $CFLAGS $test.c) 2>&1`" = ""; then
+  CFLAGS="$CFLAGS -DHAS_vsnprintf -DHAS_snprintf"
+  echo Checking for [v]snprintf support... Yes.
+else
+  echo Checking for [v]snprintf support... No.
+fi
+
+cat > $test.c <<EOF
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
diff -ur zlib-1.1.4/gzio.c zlib-1.1.4-vsnprintf/gzio.c
--- zlib-1.1.4/gzio.c	Mon Mar 11 08:16:01 2002
+++ zlib-1.1.4-vsnprintf/gzio.c	Mon Feb 24 07:48:36 2003
@@ -530,13 +530,12 @@
 
     va_start(va, format);
 #ifdef HAS_vsnprintf
-    (void)vsnprintf(buf, sizeof(buf), format, va);
+    len = vsnprintf(buf, sizeof(buf), format, va);
 #else
-    (void)vsprintf(buf, format, va);
+    len = vsprintf(buf, format, va);
 #endif
     va_end(va);
-    len = strlen(buf); /* some *sprintf don't return the nb of bytes written */
-    if (len <= 0) return 0;
+    if ((len <= 0) || (len >= Z_PRINTF_BUFSIZE)) return 0;
 
     return gzwrite(file, buf, (unsigned)len);
 }
@@ -553,14 +552,13 @@
     int len;
 
 #ifdef HAS_snprintf
-    snprintf(buf, sizeof(buf), format, a1, a2, a3, a4, a5, a6, a7, a8,
+    len = snprintf(buf, sizeof(buf), format, a1, a2, a3, a4, a5, a6, a7, a8,
 	     a9, a10, a11, a12, a13, a14, a15, a16, a17, a18, a19, a20);
 #else
-    sprintf(buf, format, a1, a2, a3, a4, a5, a6, a7, a8,
+    len = sprintf(buf, format, a1, a2, a3, a4, a5, a6, a7, a8,
 	    a9, a10, a11, a12, a13, a14, a15, a16, a17, a18, a19, a20);
 #endif
-    len = strlen(buf); /* old sprintf doesn't return the nb of bytes written */
-    if (len <= 0) return 0;
+    if ((len <= 0) || (len >= Z_PRINTF_BUFSIZE)) return 0;
 
     return gzwrite(file, buf, len);
 }