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.
>
> I believe the attached patch should solve this problem.

Thanks a lot for reporting that and writing a patch.
I've taken a slightly different approach to fixing it:
don't accept -S '' ever.
And don't recommend using gunzip -S '' in the documentation.


>From 00d2c126d60bae3bdf1a64e3fcc70355dee8f475 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 20 Feb 2010 11:07:55 +0100
Subject: [PATCH] gzip: fix a data-loss bug when decompressing with --suffix=''

* gzip.c (main): Disallow an empty --suffix=S also with -d.
Otherwise, "gzip -d -S '' F.gz" would ask if it's ok to remove the
existing file, "F.gz";  if you reply "yes", you'd lose all of that data.
Use of an empty suffix was already rejected in compression mode.
* gzip.1 (--suffix (-S)): Do not recommend to use "gunzip -S '' *".
Describe how the suffix is used when decompressing, too.
* NEWS (Bug fixes): mention the fix.
Reported by Ripduman Sohan.
---
 NEWS   |    6 ++++++
 THANKS |    1 +
 gzip.1 |   11 +++++------
 gzip.c |    6 +++---
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index 9c248f1..7a98c49 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,12 @@ GNU gzip NEWS                                    -*- outline 
-*-

 ** Bug fixes

+  "gzip -d -S '' precious.gz" is now rejected immediately.  Before,
+  that command would emulate "rm -i precious.gz", but with an easily-
+  misunderstood prompt.  I.e., gzip would ask if it's ok to remove the
+  existing file, "precious.gz".  If you made the mistake of saying "yes",
+  it would remove that input file before attempting to uncompress it.
+
   gzip -cdf now properly handles input consisting of gzip'd data followed
   by uncompressed data.  Before it would output raw compressed input, too.
   For example, now "(printf x|gzip; echo y)|gzip -dcf" prints "xy\n",
diff --git a/THANKS b/THANKS
index 183d39c..b0833e5 100644
--- a/THANKS
+++ b/THANKS
@@ -237,6 +237,7 @@ Amos Shapira            address@hidden
 Rick Sladkey            address@hidden
 Daniel L Smith          address@hidden
 Fred Smith              address@hidden
+Ripduman Sohan          address@hidden
 Stephen Soliday         address@hidden
 Paul Southworth         address@hidden
 Rob Spencer             address@hidden
diff --git a/gzip.1 b/gzip.1
index bdf6943..8db327d 100644
--- a/gzip.1
+++ b/gzip.1
@@ -291,15 +291,14 @@ will descend into the directory and compress all the 
files it finds there
 ).
 .TP
 .B \-S .suf   --suffix .suf
-Use suffix .suf instead of .gz. Any suffix can be given, but suffixes
+When compressing, use suffix .suf instead of .gz.
+Any non-empty suffix can be given, but suffixes
 other than .z and .gz should be avoided to avoid confusion when files
-are transferred to other systems.  A null suffix forces gunzip to  try
-decompression on all given files regardless of suffix, as in:
+are transferred to other systems.

-    gunzip -S "" *       (*.* for MSDOS)
+When decompressing, add .suf to the beginning of the list of
+suffixes to try, when deriving an output file name from an input file name.

-Previous versions of gzip used
-the .z suffix. This was changed to avoid a conflict with
 .IR pack "(1)".
 .TP
 .B \-t --test
diff --git a/gzip.c b/gzip.c
index 8f36bcc..9d893b3 100644
--- a/gzip.c
+++ b/gzip.c
@@ -543,11 +543,11 @@ int main (int argc, char **argv)
                 program_name);
     }
 #endif
-    if ((z_len == 0 && !decompress) || z_len > MAX_SUFFIX) {
-        fprintf(stderr, "%s: incorrect suffix '%s'\n",
-                program_name, z_suffix);
+    if (z_len == 0 || z_len > MAX_SUFFIX) {
+        fprintf(stderr, "%s: invalid suffix '%s'\n", program_name, z_suffix);
         do_exit(ERROR);
     }
+
     if (do_lzw && !decompress) work = lzw;

     /* Allocate all global buffers (for DYN_ALLOC option) */
--
1.7.0.233.g05e1a