From owner-freebsd-ports@FreeBSD.ORG Sun Aug 17 06:38:26 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 EDC7537B401; Sun, 17 Aug 2003 06:38:25 -0700 (PDT) Received: from gw.celabo.org (gw.celabo.org [208.42.49.153]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2A1AE43FBD; Sun, 17 Aug 2003 06:38:25 -0700 (PDT) (envelope-from nectar@celabo.org) Received: from madman.celabo.org (madman.celabo.org [10.0.1.111]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "madman.celabo.org", Issuer "celabo.org CA" (verified OK)) by gw.celabo.org (Postfix) with ESMTP id 9F34F54861; Sun, 17 Aug 2003 08:38:24 -0500 (CDT) Received: by madman.celabo.org (Postfix, from userid 1001) id 372B66D461; Sun, 17 Aug 2003 08:38:24 -0500 (CDT) Date: Sun, 17 Aug 2003 08:38:24 -0500 From: "Jacques A. Vidrine" To: Alexander Leidinger Message-ID: <20030817133824.GA71246@madman.celabo.org> References: <20030817130114.2bfb3cf1.Alexander@Leidinger.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030817130114.2bfb3cf1.Alexander@Leidinger.net> X-Url: http://www.celabo.org/ User-Agent: Mutt/1.5.4i-ja.1 cc: ports@freebsd.org cc: audit@freebsd.org cc: chris@aims.com.au 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: Sun, 17 Aug 2003 13:38:26 -0000 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 *)]. - 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. +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! Take care & Cheers, -- Jacques Vidrine . NTT/Verio SME . FreeBSD UNIX . Heimdal nectar@celabo.org . jvidrine@verio.net . nectar@freebsd.org . nectar@kth.se