Subject: bug#15522: closed (Re: bug#15522: gzcmp/gzdiff + gznew shell scripts use temporary files unsafely)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 04 Oct 2013 04:15:04 +0000

Your bug report

#15522: gzcmp/gzdiff + gznew shell scripts use temporary files unsafely

which was filed against the gzip package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
15522: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15522
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#15522: gzcmp/gzdiff + gznew shell scripts use temporary files unsafely
From: Paul Eggert <[email-masked]>
To: Rich Burridge <[email-masked]>
Cc: [email-masked]
Date: Thu, 03 Oct 2013 21:14:23 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

The zdiff usage of set -C is executed only on older
platforms that lack mktemp, so it shouldn't be a problem.

znew.  What a dinosaur.  It's hardly worth fixing, but I
installed this:

>From b3b5611e046b93fb20aa783d6d11d986f33f91f6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 3 Oct 2013 21:12:09 -0700
Subject: [PATCH] znew: avoid denial-of-service issue

Reported by Rich Burridge in <http://bugs.gnu.org/15522>.
* znew.in: Rewrite to avoid the need for a temporary file in /tmp.
That way, we avoid the need for set -C
and worrying about denial of service.
Use touch -r and chmod --reference rather than cpmod.
Assume cp -p works, as it's now universal.
Quote 'echo' args better, while we're at it.
(warn, tmp, cpmod, cpmodarg): Remove.
(GZIP): Unset, so that we needn't test for gzip extension.
(ext): Now always '.gz'.
* znew.1: Document the change of implementation assumptions.
---
 znew.1  | 19 +++++++++++++------
 znew.in | 51 ++++++++++++++-------------------------------------
 2 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/znew.1 b/znew.1
index dcdf84f..2a7e5e1 100644
--- a/znew.1
+++ b/znew.1
@@ -32,9 +32,16 @@ Keep a .Z file when it is smaller than the .gz file; implies
 .SH "SEE ALSO"
 gzip(1), zmore(1), zdiff(1), zgrep(1), zforce(1), gzexe(1), compress(1)
 .SH BUGS
-.I Znew
-does not maintain the time stamp with the -P option if
-.I cpmod(1)
-is not available and
-.I touch(1)
-does not support the -r option.
+If the
+.B \-P
+option is used,
+.I znew
+does not maintain the time stamp if
+.IR touch (1)
+does not support the
+.B \-r
+option, and does not maintain permissions if
+.IR chmod (1)
+does not support the
+.B \-\-reference
+option.
diff --git a/znew.in b/znew.in
index 9bd3ce9..d16311a 100644
--- a/znew.in
+++ b/znew.in
@@ -58,33 +58,9 @@ new=0
 block=1024
 # block is the disk block size (best guess, need not be exact)
 
-warn="(does not preserve modes and timestamp)"
-tmp=${TMPDIR-/tmp}/zfoo.$$
-set -C
-echo hi > $tmp || exit
-if test -z "`(${CPMOD-cpmod} $tmp $tmp) 2>&1`"; then
-  cpmod=${CPMOD-cpmod}
-  warn=""
-fi
-
-if test -z "$cpmod" && ${TOUCH-touch} -r $tmp $tmp 2>/dev/null; then
-  cpmod="${TOUCH-touch}"
-  cpmodarg="-r"
-  warn="(does not preserve file modes)"
-fi
-
-# check if GZIP env. variable uses -S or --suffix
-gzip -q $tmp
-ext=`echo $tmp* | sed "s|$tmp||"`
-rm -f $tmp*
-if test -z "$ext"; then
-  echo znew: error determining gzip extension
-  exit 1
-fi
-if test "$ext" = ".Z"; then
-  echo znew: cannot use .Z as gzip extension.
-  exit 1
-fi
+# Beware -s or --suffix in $GZIP.
+unset GZIP
+ext=.gz
 
 for arg
 do
@@ -116,26 +92,27 @@ if test -n "$opt"; then
 fi
 
 for i do
-  n=`echo $i | sed 's/.Z$//'`
+  n=`echo "$i" | sed 's/.Z$//'`
   if test ! -f "$n.Z" ; then
-    echo $n.Z not found
+    echo "$n.Z not found"
     res=1; continue
   fi
   test $keep -eq 1 && old=`wc -c < "$n.Z"`
   if test $pipe -eq 1; then
     if gzip -d < "$n.Z" | gzip $opt > "$n$ext"; then
       # Copy file attributes from old file to new one, if possible.
-      test -n "$cpmod" && $cpmod $cpmodarg "$n.Z" "$n$ext" 2> /dev/null
+      touch -r"$n.Z" -- "$n$ext" 2>/dev/null
+      chmod --reference="$n.Z" -- "$n$ext" 2>/dev/null
     else
-      echo error while recompressing $n.Z
+      echo "error while recompressing $n.Z"
       res=1; continue
     fi
   else
     if test $check -eq 1; then
-      if cp -p "$n.Z" "$n.$$" 2> /dev/null || cp "$n.Z" "$n.$$"; then
+      if cp -p "$n.Z" "$n.$$"; then
         :
       else
-        echo cannot backup "$n.Z"
+        echo "cannot backup $n.Z"
         res=1; continue
       fi
     fi
@@ -143,7 +120,7 @@ for i do
       :
     else
       test $check -eq 1 && mv "$n.$$" "$n.Z"
-      echo error while uncompressing $n.Z
+      echo "error while uncompressing $n.Z"
       res=1; continue
     fi
     if gzip $opt "$n"; then
@@ -151,10 +128,10 @@ for i do
     else
       if test $check -eq 1; then
         mv "$n.$$" "$n.Z" && rm -f "$n"
-        echo error while recompressing $n
+        echo "error while recompressing $n"
       else
         # compress $n  (might be dangerous if disk full)
-        echo error while recompressing $n, left uncompressed
+        echo "error while recompressing $n, left uncompressed"
       fi
       res=1; continue
     fi
@@ -175,7 +152,7 @@ for i do
     else
       test $pipe -eq 0 && mv "$n.$$" "$n.Z"
       rm -f "$n$ext"
-      echo error while testing $n$ext, $n.Z unchanged
+      echo "error while testing $n$ext, $n.Z unchanged"
       res=1; continue
     fi
   elif test $pipe -eq 1; then
-- 
1.8.3.1

Subject: gzcmp/gzdiff + gznew shell scripts use temporary files unsafely
From: Rich Burridge <[email-masked]>
To: [email-masked]
Date: Thu, 03 Oct 2013 17:18:57 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

We've had a bug reported against the version of gzip that we ship in 
Solaris:

"The gzcmp and gzdiff (same script hardlinked) commands shipped with 
Solaris
write to a file in the world writable directory '/tmp' if both of its
arguments are compressed files. 'set -C' is used to ensure that the file
doesn't already exist when it's being written to (which prevents a
symlink-based attack), but that allows a mild Denial of Service by creating
this file in advance, which would therefore cause gzcmp / gzdiff to abort.

                               set -C
                               trap 'rm -f /tmp/"$F".$$; exit 2' 1 2 13 
15 0
                               gzip -cdfq "$2" > /tmp/"$F".$$ || exit

gznew is similarly impacted:

       tmp=/tmp/zfoo.$$
       set -C
       echo hi > $tmp.1
       echo hi > $tmp.2

While it's arguably unlikely that these issues would ever be exploited,
it is suggested that it would be better for these commands to use mktemp."

Thanks.