From owner-freebsd-ports@FreeBSD.ORG Sun Aug 17 19:27:46 2003 Return-Path: Delivered-To: freebsd-ports@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B937937B401; Sun, 17 Aug 2003 19:27:46 -0700 (PDT) Received: from postoffice.e-easy.com.au (eth0.lnk.e-easy.com.au [203.31.73.253]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7E87543FA3; Sun, 17 Aug 2003 19:27:44 -0700 (PDT) (envelope-from chris@e-easy.com.au) Received: from postoffice.aims.com.au (nts-ts1.aims.private [192.168.10.2]) by postoffice.e-easy.com.au with ESMTP id h7I2RQLp003418; Mon, 18 Aug 2003 12:27:26 +1000 (EST) (envelope-from chris@e-easy.com.au) Received: from ntsts1 by aims.com.au (MDaemon.PRO.v6.8.4.R) with ESMTP id 35-md50000000218.tmp; Mon, 18 Aug 2003 11:57:09 +1000 From: "Chris Knight" To: "'Jacques A. Vidrine'" Date: Mon, 18 Aug 2003 11:57:08 +1000 Message-ID: <03e001c3652c$08a826f0$020aa8c0@aims.private> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook CWS, Build 9.0.2416 (9.0.2911.0) In-Reply-To: <20030817133824.GA71246@madman.celabo.org> X-MimeOLE: Produced By Microsoft MimeOLE V5.50.4925.2800 Importance: Normal X-Spam-Processed: aims.com.au, Mon, 18 Aug 2003 11:57:09 +1000 (not processed: spam filter disabled) X-Return-Path: chris@e-easy.com.au X-Virus-Scanned: by amavisd-milter (http://amavis.org/) X-Spam-Status: No, hits=-4.2 required=4.5 tests=AWL,BAYES_10,IN_REP_TO,QUOTED_EMAIL_TEXT version=2.55 X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) cc: ports@freebsd.org cc: audit@freebsd.org Subject: RE: SecFix for databases/firebird, please review X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Aug 2003 02:27:47 -0000 Howdy, > -----Original Message----- > From: owner-freebsd-ports@freebsd.org On Behalf Of Jacques > A. Vidrine > Sent: Sunday, 17 August 2003 23:38 > To: Alexander Leidinger > Cc: ports@freebsd.org; audit@freebsd.org; chris@aims.com.au > Subject: Re: SecFix for databases/firebird, please review > > > On Sun, Aug 17, 2003 at 01:01:14PM +0200, Alexander Leidinger wrote: > > Hi, > > > > at http://www.leidinger.net/FreeBSD/firebird-1.0.2-secfix.tar.bz2 > > I've some patches for the databases/firebird port (see > > http://packetstormsecurity.nl/0305-exploits/dsr-adv001.txt > > for the local stack overflow possibility). > > > > As I want to commit it to the port before Kris decides to remove > > it because it is marked FORBIDDEN since a long time, it would be > > nice if as much people as possible review the patches. > > > > Chris, it would be nice if you at least can convince the > > developers to review the patches too. And please test the patches, > > I've just verified that firebird compiles on 5-current (it needs > > one additional patch (in #ifdef'ed out code) to compile with gcc > > 3.3). > > Hallo Alexander! Thanks for giving this a shot. > > There is a lot of this: > > usr = getenv ("ISC_USER"); > + if (-1 == usr) > + { > > getenv(3) returns NULL if the given environmental variable is not set, > not -1 [char *getenv(const char *)]. > Wouldn't it be safer still to do: strlcat(usr, getenv("ISC_USER"), sizeof(usr)); > - sprintf (translated_msg_file, MSG_FILE_LANG, p); > + snprintf (translated_msg_file, sizeof (MSG_FILE_LANG) + LOCALE_MAX, > + MSG_FILE_LANG, p); > > The size argument to snprintf should be the size of the buffer; in > this case, sizeof(translated_msg_file). (Mostly harmless off-by-one > error here.) > > - strcat (string, "/"); > + strncat (string, "/", ((strlen(ib_prefix) < MAXPATHLEN) ? (MAXPATHLEN - strlen(ib_prefix)) : 0)); > > This is bogus... this function should be rewritten so that it passes > in the size of the `string' argument. One can't just assume it is > MAXPATHLEN. Also, strlcat would be much nicer and safer here. If you > can't use strlcat, then one must explicitly NUL-terminate the buffer, > because strncat may fail to do so. > That's what I'm currently in the process of doing - passing in the size of the buffer to gds__prefix. It gets called with buffer lengths of 64, 100, 128, 256 and 1024. I'm probably going to have to use strncat to keep it a bit more portable. > +TEXT file_name [33], *p, *q, *end, *end2; > > p = file_name; > +end2 = filename + sizeof (file_name); > > Er, this is almost certainly wrong. (filename vs file_name) > > +while (*q && p < end2) > *p++ = *q++; > > *p = 0; > > If the string pointed to by `q' is long enough, then when this loop > terminates `p == end2' and so `p == &file_name[sizeof(file_name)]'. > This is a single byte buffer overflow. > > Why bother trying to fix this loop, but leave the dangerous loop > immediately proceeding it? :-) > > > OK, I only looked at the first two patch files, but it is clear that > this should not be committed. IMHO, I also think this port _should_ > be removed. But, if you decide to slog through it once more and > correct some of these problems, we'll be here for another look! > I don't particularly like it, but I'm inclined to agree with you - the port probably should go. I can always maintain the 1.0.x port outside of the FreeBSD Ports Tree and make it available on my Website with lots of warning labels. I'll get onto the Firebird 1.5 port pronto, which should end this issue and put me out of my current misery. > Take care & Cheers, > -- > Jacques Vidrine . NTT/Verio SME . FreeBSD UNIX . Heimdal > nectar@celabo.org . jvidrine@verio.net . nectar@freebsd.org . > nectar@kth.se Regards, Chris Knight Systems Administrator E-Easy Tel: +61 3 6334 6664 Fax: +61 3 6331 7032 Mob: +61 419 528 795 Web: http://www.e-easy.com.au