[38382] in bugtraq
Re: gzip TOCTOU file-permissions vulnerability
daemon@ATHENA.MIT.EDU (Derek Martin)
Thu Apr 14 20:31:12 2005
Date: Wed, 13 Apr 2005 21:14:28 -0400
From: Derek Martin <code@pizzashack.org>
To: bugtraq@securityfocus.com
Cc: support@gzip.org
Message-ID: <20050414011428.GC9980@sophic.org>
Mail-Followup-To: bugtraq@securityfocus.com, support@gzip.org
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
protocol="application/pgp-signature"; boundary="fOHHtNG4YXGJ0yqR"
Content-Disposition: inline
In-Reply-To: <20050412114700.GA10763@box79162.elkhouse.de>
--fOHHtNG4YXGJ0yqR
Content-Type: multipart/mixed; boundary="gr/z0/N6AeWAPJVB"
Content-Disposition: inline
--gr/z0/N6AeWAPJVB
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
> Imran Ghory [2005-04-04 20:57 +0100]:
> > Vulnerable software
> > ====================
> >
> > gzip 1.2.4 and 1.3.3 and previous versions running on unix.
> >
> > Vulnerability
> > ==============
> >
> > If a malicious local user has write access to a directory in which a
> > target user is using gzip to extract or compress a file to then a
> > TOCTOU bug can be exploited to change the permission of any file
> > belonging to that user.
> >
> > On decompressing gzip copies the permissions from the compressed
> > gzip file to the uncompressed file. However there is a gap between the
> > uncompressed file being written (and it's file handler being close)
> > and the permissions of the file being changed.
> >
> > During this gap a malicious user can remove the decompressed file and
> > replace it with a hard-link to another file belonging to the user.
> > gzip will then change the permissions on the hard-linked file to be
> > the same as that of the gzip file.
Perusing the code seems to reveal that gzip is written this way for no
good reason... It may have been to support operating systems which
don't allow the third argument of open(), but looking at the code, the
only supported OS which doesn't support the modes argument seems to be
MacOS (presumably not MacOS X). However MacOS also seems not to
support chmod(), so there seems to be little point in separating the
two operations... The code already defines the OPEN macro which
either uses or ignores the third (modes) argument to open() as
needed.
Therefore the attached patch should apply to gzip 1.2.4 (and quite
probably to 1.3.x as well) and fix the problem.
N.B.: I didn't actually test the patch, but it looks right to me.
Yeah, I'm that lazy... =8^)
--
Derek D. Martin
http://www.pizzashack.org/
GPG Key ID: 0x81CFE75D
--gr/z0/N6AeWAPJVB
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: attachment; filename="gzip-atomic-modes.patch"
diff -ru gzip-1.2.4/gzip.c gzip-1.2.4.ddm/gzip.c
--- gzip-1.2.4/gzip.c 1993-08-19 09:39:43.000000000 -0400
+++ gzip-1.2.4.ddm/gzip.c 2005-04-13 18:13:22.404915816 -0400
@@ -279,7 +279,7 @@
local void version OF((void));
local void treat_stdin OF((void));
local void treat_file OF((char *iname));
-local int create_outfile OF((void));
+local int create_outfile OF((struct stat *istat));
local int do_stat OF((char *name, struct stat *sbuf));
local char *get_suffix OF((char *name));
local int get_istat OF((char *iname, struct stat *sbuf));
@@ -793,7 +793,7 @@
ofd = fileno(stdout);
/* keep remove_ofname as zero */
} else {
- if (create_outfile() != OK) return;
+ if (create_outfile(&istat) != OK) return;
if (!decompress && save_orig_name && !verbose && !quiet) {
fprintf(stderr, "%s: %s compressed to %s\n",
@@ -860,7 +860,8 @@
* ofname has already been updated if there was an original name.
* OUT assertions: ifd and ofd are closed in case of error.
*/
-local int create_outfile()
+local int create_outfile(istat)
+ struct stat *istat;
{
struct stat ostat; /* stat for ofname */
int flags = O_WRONLY | O_CREAT | O_EXCL | O_BINARY;
@@ -876,7 +877,7 @@
}
/* Create the output file */
remove_ofname = 1;
- ofd = OPEN(ofname, flags, RW_USER);
+ ofd = OPEN(ofname, flags, istat->st_mode & 07777);
if (ofd == -1) {
perror(ofname);
close(ifd);
@@ -1608,7 +1609,8 @@
/* ========================================================================
- * Copy modes, times, ownership from input file to output file.
+ * Copy times and ownership from input file to output file. Modes are set
+ * when the output file is created.
* IN assertion: to_stdout is false.
*/
local void copy_stat(ifstat)
@@ -1623,11 +1625,6 @@
}
reset_times(ofname, ifstat);
#endif
- /* Copy the protection modes */
- if (chmod(ofname, ifstat->st_mode & 07777)) {
- WARN((stderr, "%s: ", progname));
- if (!quiet) perror(ofname);
- }
#ifndef NO_CHOWN
chown(ofname, ifstat->st_uid, ifstat->st_gid); /* Copy ownership */
#endif
--gr/z0/N6AeWAPJVB--
--fOHHtNG4YXGJ0yqR
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQFCXcP0HEnASN++rQIRAs5bAJ0YyaevkNq3jOTGBECkncHsl2tk/wCdESGP
BBuVEDtvsPw5P8QJvyz0n+k=
=mMDP
-----END PGP SIGNATURE-----
--fOHHtNG4YXGJ0yqR--
