From owner-freebsd-audit Wed Jan 22 9:55:39 2003 Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1A63037B401 for ; Wed, 22 Jan 2003 09:55:37 -0800 (PST) Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1BA2343ED8 for ; Wed, 22 Jan 2003 09:55:36 -0800 (PST) (envelope-from keramida@freebsd.org) Received: from gothmog.gr (patr530-b161.otenet.gr [212.205.244.169]) by mailsrv.otenet.gr (8.12.6/8.12.6) with ESMTP id h0MHtV58022306 for ; Wed, 22 Jan 2003 19:55:32 +0200 (EET) Received: from gothmog.gr (gothmog [127.0.0.1]) by gothmog.gr (8.12.6/8.12.6) with ESMTP id h0MHtVxu002108 for ; Wed, 22 Jan 2003 19:55:31 +0200 (EET) (envelope-from keramida@freebsd.org) Received: (from giorgos@localhost) by gothmog.gr (8.12.6/8.12.6/Submit) id h0MHtUPj002107 for freebsd-audit@FreeBSD.ORG; Wed, 22 Jan 2003 19:55:30 +0200 (EET) (envelope-from keramida@freebsd.org) Date: Wed, 22 Jan 2003 19:55:30 +0200 From: Giorgos Keramidas To: freebsd-audit@freebsd.org Subject: Re: hopefully, a fix for an old uncompress(1) bug Message-ID: <20030122175530.GA2026@gothmog.gr> References: <20030106062309.GA37109@gothmog.gr> <20030106080949.GA382@straylight.oblivion.bg> <20030106082154.GE1094@gothmog.gr> <20030107040216.GB4071@gothmog.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030107040216.GB4071@gothmog.gr> Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I have tested the patch in the post quoted below and it seems to work around the bugs in uncompress(1). What do I need to do to get approval for commiting this or a "no, this is not good because [foo]" type of review? - Giorgos On 2003-01-07 06:02, Giorgos Keramidas wrote: > [...] > The first problem is that decompress() tries to open the output file > first and then notice that its input file (foo.Z) fails to zopen(). > When this happens, it promptly goes ahead and tries to unlink `foo' > which is supposed to be the outfile. If a file happens to be named > `foo' it is unconditionally removed. This is the behavior shown > below: > > % ls -l > -rw-rw-r-- 1 giorgos wheel - 0 Jan 6 08:04 lala > % uncompress -f lala > uncompress: lala.Z: No such file or directory > % ls -l > % > > Reversing the order of `in' and `out' file opening fixes this problem, > but there is one more which is a bit more difficult to notice. > > The second problem. If the input file of decompress() does exist but > is not a valid .Z file the zopen() call will work fine only to fail > later on when we try to fread() data from it. This is shown below: > > $ cp /COPYRIGHT lala.Z > $ touch lala > $ ls -l lala* > -rw-rw-r-- 1 giorgos giorgos - 0 Jan 7 05:42 lala > -r--r--r-- 1 giorgos giorgos - 4735 Jan 7 05:42 lala.Z > $ ./uncompress -f lala > uncompress: lala.Z: Inappropriate file type or format > $ ls -l lala* > -r--r--r-- 1 giorgos giorgos - 4735 Jan 7 05:42 lala.Z > $ > > I tried to solve this, by attempting to successfully read at least one > byte of data from the .Z file before even attempting to open the > output file. I also changed all the `goto err' parts before the > output file is opened to avoid unlinking() a file that we haven't > previously fopen()ed. > > Hopefully, this fixes both the problems :-/ > > %%% > Index: compress.c > =================================================================== > RCS file: /home/ncvs/src/usr.bin/compress/compress.c,v > retrieving revision 1.20 > diff -u -r1.20 compress.c > --- compress.c 28 Jul 2002 15:32:17 -0000 1.20 > +++ compress.c 7 Jan 2003 03:57:26 -0000 > @@ -300,14 +300,9 @@ > isreg = oreg = !exists || S_ISREG(sb.st_mode); > > ifp = ofp = NULL; > - if ((ofp = fopen(out, "w")) == NULL) { > - cwarn("%s", out); > - return; > - } > - > if ((ifp = zopen(in, "r", bits)) == NULL) { > cwarn("%s", in); > - goto err; > + return; > } > if (stat(in, &sb)) { > cwarn("%s", in); > @@ -315,6 +310,18 @@ > } > if (!S_ISREG(sb.st_mode)) > isreg = 0; > + > + /* > + * Try to read the first few uncompressed bytes from the input file > + * before blindly truncating the output file. > + */ > + if ((nr = fread(buf, 1, sizeof(buf), ifp)) == 0 || > + (ofp = fopen(out, "w")) == NULL || > + (nr != 0 && fwrite(buf, 1, nr, ofp) != nr)) { > + cwarn("%s", out); > + (void)fclose(ifp); > + return; > + } > > while ((nr = fread(buf, 1, sizeof(buf), ifp)) != 0) > if (fwrite(buf, 1, nr, ofp) != nr) { > %%% To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message