gzip -d would fail with a CRC error...

...for some inputs, and some memcpy implementations.  It is possible
that an offending input has to be compressed "from FAT filesystem
(MS-DOS, OS/2, NT)", since the sole reproducer no longer evokes a
CRC error when uncompressed and recompressed on a GNU/Linux system.
Also, using an unpatched reverse-memcpy-gzip on over 100,000 inputs
on a GNU/Linux system did not turn up another reproducer.
* inflate.c (inflate_codes): Don't call memcpy with overlapping regions.
Properly detect when source and destination overlap.
* tests/memcpy-abuse: New test, to trigger misbehavior.
* Makefile.am (TESTS): Add it.
* NEWS (Bug fixes): Mention it.
Reported by Alain Magloire in
http://thread.gmane.org/gmane.comp.gnu.gzip.bugs/307

diff --git a/Makefile.am b/Makefile.am
index c0cd415d637ed37e98f4ab27f7e5de37c0384aa8..67dc18b85dffca06e5487f44ac799a040126ef0d 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -104,6 +104,7 @@ check-local: $(FILES_TO_CHECK) $(bin_PROGRAMS) gzip.doc.gz
        @echo 'Test succeeded.'
 
 TESTS =                                                \
+  tests/memcpy-abuse                           \
   tests/trailing-nul                           \
   tests/zdiff                                  \
   tests/zgrep-f

diff --git a/NEWS b/NEWS
index 02654390c10a89943aa64d17e8032c3e72e17986..3e50762ed4853287e7c1a15b1c275554fd6a3384 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@ GNU gzip NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  gzip -d would fail with a CRC error for some valid inputs.
+  So far, the only valid input known to exhibit this failure was
+  compressed "from FAT filesystem (MS-DOS, OS/2, NT)".  In addition,
+  to trigger the failure, your memcpy implementation must copy in
+  the "reverse" order.
+
 
 * Noteworthy changes in release 1.3.14 (2009-10-30) [beta]
 

diff --git a/inflate.c b/inflate.c
index affab3742945c8cfda316bd9bf8c3941132a4e0a..5b6831434d7ecc1854365c6324322c635e9a7c5b 100644 (file)
--- a/inflate.c
+++ b/inflate.c
@@ -589,7 +589,7 @@ int bl, bd;             /* number of bits decoded by tl[] and td[] */
       do {
         n -= (e = (e = WSIZE - ((d &= WSIZE-1) > w ? d : w)) > n ? n : e);
 #if !defined(NOMEMCPY) && !defined(DEBUG)
-        if (w - d >= e)         /* (this test assumes unsigned comparison) */
+        if (d < w && w - d >= e)
         {
           memcpy(slide + w, slide + d, e);
           w += e;

diff --git a/tests/memcpy-abuse b/tests/memcpy-abuse
new file mode 100755 (executable)
index 0000000..8f2abd5
--- /dev/null
+++ b/tests/memcpy-abuse
@@ -0,0 +1,40 @@
+#!/bin/sh
+# Before gzip-1.4, this the use of memcpy in inflate_codes could
+# mistakenly operate on overlapping regions.  Exercise that code.
+
+# Copyright (C) 2010 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.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  gzip --version
+fi
+
+: ${srcdir=.}
+. "$srcdir/tests/init.sh"; path_prepend_ .
+
+# The input must be larger than 32KiB and slightly
+# less uniform than e.g., all zeros.
+printf wxy%032767d 0 | tee in | gzip > in.gz || framework_failure
+
+fail=0
+
+# Before the fix, this would call memcpy with overlapping regions.
+gzip -dc in.gz > out || fail=1
+
+compare in out || fail=1
+
+Exit $fail