Re: GZIP Decompress -- Data Loss Through File Deletion

[Jim Meyering]

Ripduman Sohan wrote:
> There's a subtle bug in gzip (1.3.13) decompression when no suffixes
> are employed.
>
> The command [gzip -d -S "" <infile>] will, as expected, ask to
> overwrite the output file.  However, it goes on to unlink the newly
> created (decompressed file) resulting in data loss through file deletion.

Thanks again.
FYI, here are two more patches to exercise the bug:

    -- tests: add ---presume-input-tty option, solely for testing
         (note the three hyphens)
    -- tests: exercise the fix for the decompression data-loss bug

>From bd0c4e025575180648388b63e7b69c6341fbd892 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 19 Feb 2010 20:52:04 +0100
Subject: [PATCH 1/2] tests: add ---presume-input-tty option, solely for testing

* gzip.c: Include <stdbool.h>.
(presume_input_tty): New global.
(main): Set it.
(treat_stdin, check_ofname): Use it.
---
 gzip.c |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/gzip.c b/gzip.c
index 141397e..8f36bcc 100644
--- a/gzip.c
+++ b/gzip.c
@@ -29,7 +29,7 @@
  */

 static char const *const license_msg[] = {
-"Copyright (C) 2007 Free Software Foundation, Inc.",
+"Copyright (C) 2007, 2010 Free Software Foundation, Inc.",
 "Copyright (C) 1993 Jean-loup Gailly.",
 "This is free software.  You may redistribute copies of it under the terms of",
 "the GNU General Public License <http://www.gnu.org/licenses/gpl.html>.",
@@ -58,6 +58,7 @@ static char const *const license_msg[] = {
 #include <ctype.h>
 #include <sys/types.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <sys/stat.h>
 #include <errno.h>

@@ -167,6 +168,10 @@ DECLARE(uch, window, 2L*WSIZE);

                 /* local variables */

+/* If true, pretend that standard input is a tty.  This option
+   is deliberately not documented, and only for testing.  */
+static bool presume_input_tty;
+
 int ascii = 0;        /* convert end-of-lines to local OS conventions */
 int to_stdout = 0;    /* output to stdout (-c) */
 int decompress = 0;   /* decompress (-d) */
@@ -243,6 +248,13 @@ static int handled_sig[] =
 #endif
   };

+/* For long options that have no equivalent short option, use a
+   non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
+enum
+{
+  PRESUME_INPUT_TTY_OPTION = CHAR_MAX + 1
+};
+
 struct option longopts[] =
 {
  /* { name  has_arg  *flag  val } */
@@ -259,6 +271,7 @@ struct option longopts[] =
     {"license",    0, 0, 'L'}, /* display software license */
     {"no-name",    0, 0, 'n'}, /* don't save or restore original name & time */
     {"name",       0, 0, 'N'}, /* save or restore original name & time */
+    {"-presume-input-tty", no_argument, NULL, PRESUME_INPUT_TTY_OPTION},
     {"quiet",      0, 0, 'q'}, /* quiet mode */
     {"silent",     0, 0, 'q'}, /* quiet mode */
     {"recursive",  0, 0, 'r'}, /* recurse through directories */
@@ -271,6 +284,7 @@ struct option longopts[] =
     {"best",       0, 0, '9'}, /* compress better */
     {"lzw",        0, 0, 'Z'}, /* make output compatible with old compress */
     {"bits",       1, 0, 'b'}, /* max number of bits per code (implies -Z) */
+
     { 0, 0, 0, 0 }
 };

@@ -468,6 +482,8 @@ int main (int argc, char **argv)
             no_name = no_time = 1; break;
         case 'N':
             no_name = no_time = 0; break;
+        case PRESUME_INPUT_TTY_OPTION:
+           presume_input_tty = true; break;
         case 'q':
             quiet = 1; verbose = 0; break;
         case 'r':
@@ -591,8 +607,9 @@ input_eof ()
  */
 local void treat_stdin()
 {
-    if (!force && !list &&
-        isatty(fileno((FILE *)(decompress ? stdin : stdout)))) {
+    if (!force && !list
+       && (presume_input_tty
+           || isatty(fileno((FILE *)(decompress ? stdin : stdout))))) {
         /* Do not send compressed data to the terminal or read it from
          * the terminal. We get here when user invoked the program
          * without parameters, so be helpful. According to the GNU standards:
@@ -1617,7 +1634,7 @@ local int check_ofname()
     if (!force) {
         int ok = 0;
         fprintf (stderr, "%s: %s already exists;", program_name, ofname);
-        if (foreground && isatty(fileno(stdin))) {
+        if (foreground && (presume_input_tty || isatty(fileno(stdin)))) {
             fprintf(stderr, " do you wish to overwrite (y or n)? ");
             fflush(stderr);
             ok = yesno();
--
1.7.0.233.g05e1a


>From fa627092978b0099297581eb90f09c7ecffc157b Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 19 Feb 2010 20:50:09 +0100
Subject: [PATCH 2/2] tests: exercise the fix for the decompression data-loss bug

* tests/null-suffix-clobber: New file.
* Makefile.am (TESTS): Add it.
---
 Makefile.am               |    1 +
 tests/null-suffix-clobber |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100755 tests/null-suffix-clobber

diff --git a/Makefile.am b/Makefile.am
index 816e9f4..3b048b8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -108,6 +108,7 @@ TESTS =                                             \
   tests/hufts                                  \
   tests/memcpy-abuse                           \
   tests/mixed                                  \
+  tests/null-suffix-clobber                    \
   tests/stdin                                  \
   tests/trailing-nul                           \
   tests/zdiff                                  \
diff --git a/tests/null-suffix-clobber b/tests/null-suffix-clobber
new file mode 100755
index 0000000..e5b52a1
--- /dev/null
+++ b/tests/null-suffix-clobber
@@ -0,0 +1,41 @@
+#!/bin/sh
+# Before gzip-1.5, gzip -d -S '' k.gz would delete F.gz and not create "F"
+
+# 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"
+
+printf anything | gzip > F.gz || framework_failure
+echo y > yes || framework_failure
+echo "gzip: invalid suffix ''" > expected-err || framework_failure
+
+fail=0
+
+gzip ---presume-input-tty -d -S '' F.gz < yes > out 2>err && fail=1
+
+compare out /dev/null || fail=1
+compare err expected-err || fail=1
+
+test -f F.gz || fail=1
+
+Exit $fail
--
1.7.0.233.g05e1a