[PATCH] maint: prohibit compilation with bug-evoking cpp definitions

[Jim Meyering]

FYI,

>From 3a7db5b4a524f62a15a7d2f47664787442bf2c0b Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 12 Aug 2011 14:23:40 +0200
Subject: [PATCH] maint: prohibit compilation with bug-evoking cpp definitions

gzip is designed to be compiled with -DSMALL_MEM or -DDIST_BUFSIZE=N,
primarily to accommodate older systems with very limited memory.
However, compiling with -DSMALL_MEM produces a gzip program that
overflows a stack buffer when LZW-uncompressing some files.
It appears that no one builds GNU gzip with either symbol defined
these days.  Hence, this change is solely to avoid the risk that
someone would experiment with these settings and use the resulting
binary.
* gzip.h: Ensure that this code is never compiled with
-DSMALL_MEM or -DDIST_BUFSIZE=N, since those can lead to
stack buffer overflow.
* unlzw.c (unlzw): Add a commented out assertion that would fail
when compiled with -DSMALL_MEM and running the small-mem test.
* tests/small-mem: New test.
* tests/Makefile.am (TESTS): Add it.
Thanks to Tomas Hoger for the analysis, providing the asserted
expression and the test-writing hint that I'd probably need
.Z-compressed 200MiB of /dev/zero to trigger the misbehavior.
While stack overflow does occur with a 200MiB input, input/output
mismatch required more like 250MiB.  Here are related URLs:
http://bugzilla.redhat.com/CVE-2011-2895
http://bugzilla.redhat.com/CVE-2011-2896
---
 gzip.h            |    7 +++++++
 tests/Makefile.am |    1 +
 tests/small-mem   |   42 ++++++++++++++++++++++++++++++++++++++++++
 unlzw.c           |    2 ++
 4 files changed, 52 insertions(+), 0 deletions(-)
 create mode 100755 tests/small-mem

diff --git a/gzip.h b/gzip.h
index 568ffb0..8db18ff 100644
--- a/gzip.h
+++ b/gzip.h
@@ -69,6 +69,13 @@ typedef unsigned long  ulg;
 #define MAX_METHODS 9
 extern int method;         /* compression method */

+/* Avoid risk of stack overflow.
+   Defining SMALL_MEM or defining DIST_BUFSIZE to a value that
+   is too small would lead to a stack buffer overflow in unlzw.  */
+#if defined SMALL_MEM || defined DIST_BUFSIZE
+# error "do not define SMALL_MEM or DIST_BUFSIZE"
+#endif
+
 /* To save memory for 16 bit systems, some arrays are overlaid between
  * the various modules:
  * deflate:  prev+head   window      d_buf  l_buf  outbuf
diff --git a/tests/Makefile.am b/tests/Makefile.am
index cb7f94a..a09e2ac 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,6 +21,7 @@ TESTS =                                       \
   memcpy-abuse                         \
   mixed                                        \
   null-suffix-clobber                  \
+  small-mem                            \
   stdin                                        \
   trailing-nul                         \
   zdiff                                        \
diff --git a/tests/small-mem b/tests/small-mem
new file mode 100755
index 0000000..3dfa547
--- /dev/null
+++ b/tests/small-mem
@@ -0,0 +1,42 @@
+#!/bin/sh
+# With gzip-1.3.14 and prior, compiled with -DSMALL_MEM, or with
+# -DDIST_BUFSIZE=N for too small an N, uncompressing valid LZW data
+# (often with .Z suffix, created by e.g., compress/ncompress) could
+# result in stack overflow and (at least) invalid output.
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+# limit so don't run it by default.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ..
+
+# Require a compress program.
+echo aaaaaaaaa > t && compress t
+test -f t.Z || skip_ 'this test requires the compress program'
+
+# Create a 300MiB input file, all NUL bytes.
+dd bs=1048576 count=300 if=/dev/zero > in
+compress < in > in.Z || framework_failure_
+
+fail=0
+
+gzip -dc in.Z > out || fail=1
+
+# When gzip is compiled with -DSMALL_MEM, "out" ends up containing
+# only 243501410 bytes.
+cmp out in || fail=1
+wc -c in out
+
+Exit $fail
diff --git a/unlzw.c b/unlzw.c
index 63f941c..537b8ed 100644
--- a/unlzw.c
+++ b/unlzw.c
@@ -324,6 +324,8 @@ int unlzw(in, out)
                 /* Generate output characters in reverse order */
                 *--stackp = tab_suffixof(code);
                 code = tab_prefixof(code);
+                /* Avoid stack buffer overrun with -DSMALL_MEM: */
+                /* assert (((char_type *)(&d_buf[0])) < stackp); */
             }
             *--stackp =        (char_type)(finchar = tab_suffixof(code));

--
1.7.6.433.g1421f