Date: Wed, 22 Jan 2003 19:55:30 +0200 From: Giorgos Keramidas <keramida@freebsd.org> To: freebsd-audit@freebsd.org Subject: Re: hopefully, a fix for an old uncompress(1) bug Message-ID: <20030122175530.GA2026@gothmog.gr> In-Reply-To: <20030107040216.GB4071@gothmog.gr> References: <20030106062309.GA37109@gothmog.gr> <20030106080949.GA382@straylight.oblivion.bg> <20030106082154.GE1094@gothmog.gr> <20030107040216.GB4071@gothmog.gr>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <keramida@FreeBSD.ORG> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030122175530.GA2026>