")) != NULL) { > char *endtitle; > char titletemp[256]; > > titlep += 7; > if ((endtitle = strcasestr(titlep, "

To: gopher@complete.org, control@bugs.debian.org,
	82602@bugs.debian.org
Subject: [gopher] Re: Fwd: Bug#82602: gopherd: [SECURITY] gopherd is dangerous
References: <20010116231004.A19307@vitelus.com>
From: John Goerzen <jgoerzen@complete.org>
Date: 17 Jan 2001 11:31:57 -0500
In-Reply-To: <20010116231004.A19307@vitelus.com>
Message-ID: <87g0ii16o2.fsf@complete.org>
Lines: 113
User-Agent: Gnus/5.090001 (Oort Gnus v0.01) XEmacs/21.1 (Channel Islands)
MIME-Version: 1.0
Content-type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 8bit
Sender: gopher-bounce@complete.org
Errors-to: gopher-bounce@complete.org
Precedence: bulk
Reply-to: gopher@complete.org


severity 82602 normal
thanks

The mere existance of sprintf, strcpy, and strcat does not mean that
there is a bug.  If the data being used is already of a known size,
and that size is less than or equal to the location it is going, there
is no problem.  Therefore, the grep is meaningless.

For the rest, please provide specific file/line number references so
that they can be checked to see if there is really a bug there or not.

-- John

Aaron Lehmann <aaronl@vitelus.com> writes:

> From: aaronl@vitelus.com
> Subject: Bug#82602: gopherd: [SECURITY] gopherd is dangerous
> To: submit@bugs.debian.org
> Date: Tue, 16 Jan 2001 22:57:23 -0800
> 
> Package: gopherd
> Version: 2.3.1-8
> Severity: grave
> 
> 
> First off:
> 
> $ egrep -r '(sprintf|strcpy|strcat)' * | wc -l
>     539
> 
> *shudder*
> 
> 
> Here are a few particular cases of fixed-size buffers that I think may
> currently be security risks:
> 
>      char buf[256];
> ...
>       if (dochroot)
>            sprintf(buf, "%s '%s'", decoder, pathname);
>       else
>            sprintf(buf, "%s '%s/%s'", decoder, Data_Dir, pathname);
> 
> As far as I can tell, neither decoder nor pathname is regulated in
> size at all.
> 
> Here's another favorite:
>      char         longname[256];
> ...
>            sprintf( longname, "%s  [%s%s%s, %ukb]", stitle,
>               cdate+8,cdate+4,cdate+22, (statbuf.st_size+1023) / 1024);
> 
> Even if the length of stitle was regulated (which I doubt), it would
> most likely be regulated to 256 bytes, which would be just as
> disasterous.
> 
> Oh, and you had better hope that the path to your Data_Dir is < 256 chars:
>      char tmpstr[256];
> ...
>             strcpy(tmpstr, Data_Dir);
> 
> Data_Dir is _not_ regulated in size:
>       Data_Dir = strdup(argv[optind]);
> ...
>       Data_Dir = strdup(DATA_DIRECTORY);
> 
> How about this:
> 
>      if ((titlep = strcasestr(buf, "<TITLE>")) != NULL) {
>       char *endtitle;
>       char titletemp[256];
> 
>       titlep += 7;
>       if ((endtitle = strcasestr(titlep, "</TITLE>")) != NULL) {
>            strncpy(titletemp, titlep, (endtitle-titlep));
>            titletemp[endtitle-titlep] = '\0';
> 
> So, list a directory containing a .html document with a title > 256
> chars and you're likely to smash the stack.
> 
> I could go on and on. My reccomendation to the gopherd maintainer is
> to throw out all of this code and write a more modern, secure
> implentation from scratch. This is the worst C code I have ever read.
> 
> 
> --  
> To UNSUBSCRIBE, email to debian-bugs-dist-request@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
> 
> 
> 
> ----------
> 
> 
> -- Attached file included as plaintext by Listar --
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.0.4 (GNU/Linux)
> Comment: For info see http://www.gnupg.org
> 
> iD8DBQE6ZUVMdtqQf66JWJkRAkfcAKC+DYo7IlV/uMhb9TiNFMehmoqDhQCfWdSG
> D5NRK+qja4sbChxnEeh4m10=
> =+VYC
> -----END PGP SIGNATURE-----
> 
> 
> 
> 

-- 
John Goerzen <jgoerzen@complete.org>                       www.complete.org
Sr. Software Developer, Progeny Linux Systems, Inc.    www.progenylinux.com
#include <std_disclaimer.h>                     <jgoerzen@progenylinux.com>