GNU bug report logs -
#15522
gzcmp/gzdiff + gznew shell scripts use temporary files unsafely
Previous Next
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
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 15522 <at> debbugs.gnu.org.
--
15522: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15522
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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 <eggert <at> cs.ucla.edu>
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
[Message part 3 (message/rfc822, inline)]
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.
This bug report was last modified 11 years and 254 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.