> 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
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
Attachment:
pgpDRufmrwgZC.pgp
Description: PGP signature