7730384 2001-12-30 14:26 +0000 /28 rader/ greg <gregn@dekode.org>
Sänt av: joel@lysator.liu.se
Importerad: 2001-12-31 02:47 av Brevbäraren
Extern mottagare: bugtraq@securityfocus.com
Mottagare: Bugtraq (import) <20317>
Ärende: gzip bug w/ patch..
------------------------------------------------------------
From: "greg" <gregn@dekode.org>
To: <bugtraq@securityfocus.com>
Message-ID: <001901c1913d$edb9e4a0$3fdaf30c@c534987-a.attbi.com>
Earlier, Goobles had pointed out a bug in Gzip pertaining to this
code in (gzip.c):
if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
strcpy(nbuf,dir);
if (len != 0 /* dir = "" means current dir on Amiga */
#ifdef PATH_SEP2
while looking through I have found that the real problem lied here in
(gzip.c):
line 1009:
strcpy(ifname, iname);
well anyway, there is an attached patch, bye.
(7730384) /greg <gregn@dekode.org>/-------(Ombruten)
Bilaga (application/octet-stream) i text 7730385
Kommentar i text 7732768 av Tim J. Robbins <tim@robbins.dropbear.id.au>
Kommentar i text 7732780 av Wojtek Pilorz <wpilorz@bdk.pl>
7730385 2001-12-30 14:26 +0000 /21 rader/ greg <gregn@dekode.org>
Bilagans filnamn: "gzip.patch"
Importerad: 2001-12-31 02:47 av Brevbäraren
Extern mottagare: bugtraq@securityfocus.com
Mottagare: Bugtraq (import) <20318>
Bilaga (text/plain) till text 7730384
Ärende: Bilaga (gzip.patch) till: gzip bug w/ patch..
------------------------------------------------------------
--- gzip.c Thu Aug 19 09:39:43 1993
+++ gzip-fix.c Sun Dec 30 13:57:44 2001
@@ -1006,7 +1006,7 @@
char *dot; /* pointer to ifname extension, or NULL */
#endif
- strcpy(ifname, iname);
+ strncpy(ifname, iname, sizeof(ifname) - 1);
/* If input file exists, return OK. */
if (do_stat(ifname, sbuf) == 0) return OK;
@@ -1683,7 +1683,7 @@
}
len = strlen(dir);
if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
- strcpy(nbuf,dir);
+ strncpy(nbuf, dir, sizeof(nbuf) - 1);
if (len != 0 /* dir = "" means current dir on Amiga */
#ifdef PATH_SEP2
&& dir[len-1] != PATH_SEP2
(7730385) /greg <gregn@dekode.org>/-----------------
7732768 2001-12-31 13:05 +1100 /19 rader/ Tim J. Robbins <tim@robbins.dropbear.id.au>
Sänt av: joel@lysator.liu.se
Importerad: 2002-01-01 00:23 av Brevbäraren
Extern mottagare: bugtraq@securityfocus.com
Mottagare: Bugtraq (import) <20327>
Kommentar till text 7730384 av greg <gregn@dekode.org>
Ärende: Re: gzip bug w/ patch..
------------------------------------------------------------
From: "Tim J. Robbins" <tim@robbins.dropbear.id.au>
To: bugtraq@securityfocus.com
Message-ID: <20011231130538.B9804@raven.robbins.dropbear.id.au>
On Sun, Dec 30, 2001 at 02:26:10PM -0000, greg wrote:
> well anyway, there is an attached patch, bye.
> - strcpy(nbuf,dir);
> + strncpy(nbuf, dir, sizeof(nbuf) - 1);
You must ensure the trailing NUL character is at the end of the
string:
nbuf[sizeof(nbuf) - 1] = '\0';
Tim
(7732768) /Tim J. Robbins <tim@robbins.dropbear.id.au>/
7732780 2001-12-31 10:40 +0100 /89 rader/ Wojtek Pilorz <wpilorz@bdk.pl>
Sänt av: joel@lysator.liu.se
Importerad: 2002-01-01 00:43 av Brevbäraren
Extern mottagare: greg <gregn@dekode.org>
Extern kopiemottagare: bugtraq@securityfocus.com
Extern kopiemottagare: gzip@gnu.org
Extern kopiemottagare: jloup@chorus.fr
Mottagare: Bugtraq (import) <20329>
Kommentar till text 7730384 av greg <gregn@dekode.org>
Ärende: Re: gzip bug w/ patch..
------------------------------------------------------------
From: Wojtek Pilorz <wpilorz@bdk.pl>
To: greg <gregn@dekode.org>
Cc: bugtraq@securityfocus.com, gzip@gnu.org, jloup@chorus.fr
Message-ID: <Pine.LNX.4.21.0112310922080.674-100000@celebris.bdk.pl>
Greg,
strncpy in the patch you have attached might not include 0 terminator,
which seems dangerous.
I think it would be preferred to follow
strncpy(ifname, iname, sizeof(ifname) - 1);
with
ifname[sizeof(ifname) - 1] = 0;
Whether too long names should be quietly truncated and then accepted
is another problem - I guess they should not (esp. since the path
might be later modified, e.g. by appending .gz) In any case, the
first part of the patch does not seem to go far enough, since there
are other strcpy/strcat calls below line 1009 in get_istat function.
On the other hand, a quick look at gzip-1.2.4a/gzip.c, lines 1683 and
following shows that the second part of patch is not needed, since
sizeof(nbuf) is MAX_PATH_LEN and the if condition at line 1685
would prevent strcpy from being performed if 'dir' is too long.
Thank you for drawing our attention to this gzip problem.
Best regards,
Wojtek
--------------------
Wojtek Pilorz
Wojtek.Pilorz@bdk.pl
On Sun, 30 Dec 2001, greg wrote:
> Date: Sun, 30 Dec 2001 14:26:10 -0000
> From: greg <gregn@dekode.org>
> To: bugtraq@securityfocus.com
> Subject: gzip bug w/ patch..
>
> Earlier, Goobles had pointed out a bug in Gzip pertaining to this code in
> (gzip.c):
>
> if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
> strcpy(nbuf,dir);
> if (len != 0 /* dir = "" means current dir on Amiga */
> #ifdef PATH_SEP2
>
>
> while looking through I have found that the real problem lied here in
> (gzip.c):
>
> line 1009:
>
> strcpy(ifname, iname);
>
>
> well anyway, there is an attached patch, bye.
>
>
>
>
>
> --- gzip.c Thu Aug 19 09:39:43 1993
> +++ gzip-fix.c Sun Dec 30 13:57:44 2001
> @@ -1006,7 +1006,7 @@
> char *dot; /* pointer to ifname extension, or NULL */
> #endif
>
> - strcpy(ifname, iname);
> + strncpy(ifname, iname, sizeof(ifname) - 1);
>
> /* If input file exists, return OK. */
> if (do_stat(ifname, sbuf) == 0) return OK;
> @@ -1683,7 +1683,7 @@
> }
> len = strlen(dir);
> if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
> - strcpy(nbuf,dir);
> + strncpy(nbuf, dir, sizeof(nbuf) - 1);
> if (len != 0 /* dir = "" means current dir on Amiga */
> #ifdef PATH_SEP2
> && dir[len-1] != PATH_SEP2
(7732780) /Wojtek Pilorz <wpilorz@bdk.pl>/(Ombruten)