From owner-freebsd-audit@FreeBSD.ORG Sun Aug 17 04:00:13 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DFE4E37B401; Sun, 17 Aug 2003 04:00:13 -0700 (PDT) Received: from mailout06.sul.t-online.com (mailout06.sul.t-online.com [194.25.134.19]) by mx1.FreeBSD.org (Postfix) with ESMTP id 54BA643F3F; Sun, 17 Aug 2003 04:00:10 -0700 (PDT) (envelope-from Alexander@Leidinger.net) Received: from fwd06.aul.t-online.de by mailout06.sul.t-online.com with smtp id 19oLGU-0003yb-00; Sun, 17 Aug 2003 13:00:02 +0200 Received: from Andro-Beta.Leidinger.net (r2M-hBZZweSv8DC6xnB9NdQj9rMQyy0IbPOrkAyVnCLt3TZ3sVOkgJ@[217.83.23.54]) by fmrl06.sul.t-online.com with esmtp id 19oLGL-0GT0am0; Sun, 17 Aug 2003 12:59:53 +0200 Received: from Magelan.Leidinger.net (Magelan [192.168.1.1]) h7HB0s9O040266; Sun, 17 Aug 2003 13:00:54 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Received: from Magelan.Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.9/8.12.9) with SMTP id h7HB1EiZ022762; Sun, 17 Aug 2003 13:01:14 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Date: Sun, 17 Aug 2003 13:01:14 +0200 From: Alexander Leidinger To: audit@freebsd.org Message-Id: <20030817130114.2bfb3cf1.Alexander@Leidinger.net> X-Mailer: Sylpheed version 0.9.3claws (GTK+ 1.2.10; i386-portbld-freebsd5.1) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Seen: false X-ID: r2M-hBZZweSv8DC6xnB9NdQj9rMQyy0IbPOrkAyVnCLt3TZ3sVOkgJ@t-dialin.net cc: ports@freebsd.org cc: chris@aims.com.au Subject: SecFix for databases/firebird, please review X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Aug 2003 11:00:14 -0000 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). Bye, Alexander. -- To boldly go where I surely don't belong. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 From owner-freebsd-audit@FreeBSD.ORG Sun Aug 17 06:38:25 2003 Return-Path: Delivered-To: freebsd-audit@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-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit 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 From owner-freebsd-audit@FreeBSD.ORG Sun Aug 17 19:27:46 2003 Return-Path: Delivered-To: freebsd-audit@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-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit 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 From owner-freebsd-audit@FreeBSD.ORG Mon Aug 18 02:58:27 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C0EFB37B401; Mon, 18 Aug 2003 02:58:18 -0700 (PDT) Received: from mailout03.sul.t-online.com (mailout03.sul.t-online.com [194.25.134.81]) by mx1.FreeBSD.org (Postfix) with ESMTP id 90CB843F85; Mon, 18 Aug 2003 02:58:16 -0700 (PDT) (envelope-from Alexander@Leidinger.net) Received: from fwd04.aul.t-online.de by mailout03.sul.t-online.com with smtp id 19ogmE-0000yc-03; Mon, 18 Aug 2003 11:58:14 +0200 Received: from Andro-Beta.Leidinger.net (STlpIZZSgekfTX7hwFksnjd8Ts3fHEst0bHSTMJsvWuCoInTkIkzwC@[217.229.223.151]) by fmrl04.sul.t-online.com with esmtp id 19ogm5-05I3LU0; Mon, 18 Aug 2003 11:58:05 +0200 Received: from Magelan.Leidinger.net (Magelan [192.168.1.1]) h7I9x69O044614; Mon, 18 Aug 2003 11:59:06 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Received: from Magelan.Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.9/8.12.9) with SMTP id h7I9xSZJ062009; Mon, 18 Aug 2003 11:59:28 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Date: Mon, 18 Aug 2003 11:59:28 +0200 From: Alexander Leidinger To: "Jacques A. Vidrine" Message-Id: <20030818115928.20c1c570.Alexander@Leidinger.net> In-Reply-To: <20030817133824.GA71246@madman.celabo.org> References: <20030817130114.2bfb3cf1.Alexander@Leidinger.net> <20030817133824.GA71246@madman.celabo.org> X-Mailer: Sylpheed version 0.9.3claws (GTK+ 1.2.10; i386-portbld-freebsd5.1) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Seen: false X-ID: STlpIZZSgekfTX7hwFksnjd8Ts3fHEst0bHSTMJsvWuCoInTkIkzwC@t-dialin.net cc: ports@freebsd.org cc: audit@freebsd.org cc: Chris Knight Subject: Re: SecFix for databases/firebird, please review X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Aug 2003 09:58:27 -0000 On Sun, 17 Aug 2003 08:38:24 -0500 "Jacques A. Vidrine" wrote: > 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 *)]. Ooops... I misread the "setenv" in the "RETURN VALUES" section as "getenv"... > - 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.) Fixed. > - 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 Chris is in the process of doing this. But so far I've only seen places where MAXPATHLEN is a valid assumption (generally I agree to your suggestion, but here it seems to be a good workaround; I want to have a minimal patch, I do not want to rewrite firebird). > 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. We want to have the patch accepted by the upstream, so we shouldn't use strlcat IMHO. ---snip--- The strncat() function appends not more than count characters from append, and then adds a terminating `\0'. ---snip--- I think we have to reword the man-page then. The strncpy man-page explains it better (for strncpy). From reading the strncat man-page I assumed the string is always terminated. > +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) Argh... fixed. > +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. Fixed. > Why bother trying to fix this loop, but leave the dangerous loop > immediately proceeding it? :-) The one with "q = TEMP_PATTERN"? It's an internal buffer so I assume it isn't greater than the destination buffer. I want to fix the local exploit which involves getenv(). I just added the appropriate check to make you feel better. > OK, I only looked at the first two patch files, but it is clear that > this should not be committed. Yes. > IMHO, I also think this port _should_ > be removed. To exploit this bug you need a shell account on the machine. If it is a dedicated DB machine and only a limited set of trusted people have access to it, firebird can be used. So why remove it? > But, if you decide to slog through it once more and > correct some of these problems, we'll be here for another look! Thanks for the review. I've updated http://www.leidinger.net/FreeBSD/firebird-1.0.2-secfix.tar.bz2 (modulo Chris' work in progress). I'm looking forward to the next round. :-) Bye, Alexander. -- Yes, I've heard of "decaf." What's your point? http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 From owner-freebsd-audit@FreeBSD.ORG Mon Aug 18 04:19:01 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 111B337B401; Mon, 18 Aug 2003 04:19:01 -0700 (PDT) Received: from mailout02.sul.t-online.com (mailout02.sul.t-online.com [194.25.134.17]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9B4DF4419C; Mon, 18 Aug 2003 04:18:59 -0700 (PDT) (envelope-from Alexander@Leidinger.net) Received: from fwd02.aul.t-online.de by mailout02.sul.t-online.com with smtp id 19oi2A-0002FH-00; Mon, 18 Aug 2003 13:18:46 +0200 Received: from Andro-Beta.Leidinger.net (TluYfvZCwed-Om9P+yNM3KidWJqYUdQJ7A9v6gzYzKcbG0-SPZCAZ4@[217.229.223.151]) by fmrl02.sul.t-online.com with esmtp id 19oi1t-1y6XT60; Mon, 18 Aug 2003 13:18:29 +0200 Received: from Magelan.Leidinger.net (Magelan [192.168.1.1]) h7IBJT9O044860; Mon, 18 Aug 2003 13:19:29 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Received: from Magelan.Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.9/8.12.9) with SMTP id h7IBJpZJ062562; Mon, 18 Aug 2003 13:19:51 +0200 (CEST) (envelope-from Alexander@Leidinger.net) Date: Mon, 18 Aug 2003 13:19:51 +0200 From: Alexander Leidinger To: "Chris Knight" Message-Id: <20030818131951.5690fa0e.Alexander@Leidinger.net> In-Reply-To: <03e001c3652c$08a826f0$020aa8c0@aims.private> References: <20030817133824.GA71246@madman.celabo.org> <03e001c3652c$08a826f0$020aa8c0@aims.private> X-Mailer: Sylpheed version 0.9.3claws (GTK+ 1.2.10; i386-portbld-freebsd5.1) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Seen: false X-ID: TluYfvZCwed-Om9P+yNM3KidWJqYUdQJ7A9v6gzYzKcbG0-SPZCAZ4@t-dialin.net cc: ports@freebsd.org cc: audit@freebsd.org Subject: Re: SecFix for databases/firebird, please review X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Aug 2003 11:19:01 -0000 On Mon, 18 Aug 2003 11:57:08 +1000 "Chris Knight" wrote: > > 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. Ugh... seems I've missed some calls... > I'm probably going to have to use strncat to keep it a bit more > portable. That's the reason why I haven't used strlcat... > > 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 We can add the warning labels also to the in tree port... > should end this issue and put me out of my current misery. And you're sure 1.5 is better in this regard? Bye, Alexander. -- Give a man a fish and you feed him for a day; teach him to use the Net and he won't bother you for weeks. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 From owner-freebsd-audit@FreeBSD.ORG Mon Aug 18 05:09:36 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3A18737B401; Mon, 18 Aug 2003 05:09:36 -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 F081643F93; Mon, 18 Aug 2003 05:09:33 -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 h7IC9Npp074198; Mon, 18 Aug 2003 22:09:23 +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 26-md50000000223.tmp; Mon, 18 Aug 2003 21:38:40 +1000 From: "Chris Knight" To: "'Alexander Leidinger'" Date: Mon, 18 Aug 2003 21:38:39 +1000 Message-ID: <040d01c3657d$4566d3b0$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: <20030818131951.5690fa0e.Alexander@Leidinger.net> X-MimeOLE: Produced By Microsoft MimeOLE V5.50.4925.2800 Importance: Normal X-Spam-Processed: aims.com.au, Mon, 18 Aug 2003 21:38:40 +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-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Aug 2003 12:09:36 -0000 Howdy, > -----Original Message----- > From: owner-freebsd-audit@freebsd.org On Behalf Of Alexander Leidinger > Sent: Monday, 18 August 2003 21:20 > To: Chris Knight > Cc: ports@freebsd.org; audit@freebsd.org > Subject: Re: SecFix for databases/firebird, please review > > > On Mon, 18 Aug 2003 11:57:08 +1000 > "Chris Knight" wrote: > > > > [snip] > > 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. > > Ugh... seems I've missed some calls... > Yeah, it's not an easy fix, unfortunately :-( > > I'm probably going to have to use strncat to keep it a bit more > > portable. > > That's the reason why I haven't used strlcat... > Cool. > > 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 > > We can add the warning labels also to the in tree port... > Possibly, but if Jacques or Kris insist on it going, then I'm not going to waste my time and theirs arguing about it. > > should end this issue and put me out of my current misery. > > And you're sure 1.5 is better in this regard? > Yes, just. > Bye, > Alexander. > 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 From owner-freebsd-audit@FreeBSD.ORG Thu Aug 21 14:09:02 2003 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7617016A4BF for ; Thu, 21 Aug 2003 14:09:02 -0700 (PDT) Received: from mgr6.xmission.com (mgr6.xmission.com [198.60.22.206]) by mx1.FreeBSD.org (Postfix) with ESMTP id 94DFF43FBD for ; Thu, 21 Aug 2003 14:09:01 -0700 (PDT) (envelope-from glewis@eyesbeyond.com) Received: from mail by mgr6.xmission.com with spam-scanned (Exim 3.35 #1) id 19pwfz-0006DJ-06 for freebsd-audit@freebsd.org; Thu, 21 Aug 2003 15:09:01 -0600 Received: from [207.135.128.145] (helo=misty.eyesbeyond.com) by mgr6.xmission.com with esmtp (Exim 3.35 #1) id 19pwfx-0006Bs-06; Thu, 21 Aug 2003 15:08:57 -0600 Received: from misty.eyesbeyond.com (localhost.eyesbeyond.com [127.0.0.1]) by misty.eyesbeyond.com (8.12.9/8.12.9) with ESMTP id h7LLHYC5065114; Thu, 21 Aug 2003 15:17:34 -0600 (MDT) (envelope-from glewis@eyesbeyond.com) Received: (from glewis@localhost) by misty.eyesbeyond.com (8.12.9/8.12.9/Submit) id h7LLHWo1065110; Thu, 21 Aug 2003 15:17:32 -0600 (MDT) X-Authentication-Warning: misty.eyesbeyond.com: glewis set sender to glewis@eyesbeyond.com using -f Date: Thu, 21 Aug 2003 15:17:31 -0600 From: Greg Lewis To: freebsd-audit@freebsd.org Message-ID: <20030821211731.GA61715@misty.eyesbeyond.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="nFreZHaLTZJo0R7j" Content-Disposition: inline User-Agent: Mutt/1.4.1i X-Spam-Status: No, hits=-8.4 required=8.0 tests=BAYES_10,PATCH_UNIFIED_DIFF,USER_AGENT_MUTT,X_AUTH_WARNING autolearn=ham version=2.55 X-Spam-Level: X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) Subject: Fix buffer overflow in rogue (misc/43886) X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Aug 2003 21:09:02 -0000 --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi all, Restoring save files for rogue(6) has a potential buffer overflow allowing the attacker to gain gid games. See PR misc/43886. As penance for enabling rogue in the freebsd-games port I've ported over the NetBSD fixes for this buffer overflow and committed them to the port. However, rogue is still unpatched in -STABLE. The attached patch addresses the problem in -STABLE. In addition, it includes the spelling fix from bin/45701. Note that I've only ported over the buffer overflow fix, not the rest of the code cleanup which was done in NetBSD. I'd appreciate it if someone could review the fix and preferably commit it before the imminent 4.9 code freeze (as it is a security fix, albeit a relatively minor one in the scheme of things). -- Greg Lewis Email : glewis@eyesbeyond.com Eyes Beyond Web : http://www.eyesbeyond.com Information Technology FreeBSD : glewis@FreeBSD.org --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="rogue.diff" Index: rogue/inventory.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/inventory.c,v retrieving revision 1.4 diff -u -r1.4 inventory.c --- rogue/inventory.c 30 Nov 1999 03:49:23 -0000 1.4 +++ rogue/inventory.c 21 Aug 2003 19:58:45 -0000 @@ -415,14 +415,14 @@ mix_colors() { short i, j, k; - char *t; + char t[MAX_ID_TITLE_LEN]; for (i = 0; i <= 32; i++) { j = get_rand(0, (POTIONS - 1)); k = get_rand(0, (POTIONS - 1)); - t = id_potions[j].title; - id_potions[j].title = id_potions[k].title; - id_potions[k].title = t; + memcpy(t, id_potions[j].title, MAX_ID_TITLE_LEN); + memcpy(id_potions[j].title, id_potions[k].title, MAX_ID_TITLE_LEN); + memcpy(id_potions[k].title, t, MAX_ID_TITLE_LEN); } } Index: rogue/message.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/message.c,v retrieving revision 1.7.2.1 diff -u -r1.7.2.1 message.c --- rogue/message.c 20 Jul 2000 10:35:07 -0000 1.7.2.1 +++ rogue/message.c 21 Aug 2003 19:58:51 -0000 @@ -60,7 +60,7 @@ char msgs[NMESSAGES][DCOLS] = {"", "", "", "", ""}; short msg_col = 0, imsg = -1; boolean msg_cleared = 1, rmsg = 0; -char hunger_str[8] = ""; +char hunger_str[HUNGER_STR_LEN] = ""; const char *more = "-more-"; extern boolean cant_int, did_int, interrupted, save_is_interactive, flush; Index: rogue/move.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/move.c,v retrieving revision 1.7 diff -u -r1.7 move.c --- rogue/move.c 30 Nov 1999 03:49:24 -0000 1.7 +++ rogue/move.c 21 Aug 2003 20:02:38 -0000 @@ -64,7 +64,6 @@ extern short cur_level, max_level; extern short bear_trap, haste_self, confused; extern short e_rings, regeneration, auto_search; -extern char hunger_str[]; extern boolean being_held, interrupted, r_teleport, passgo; one_move_rogue(dirch, pickup) Index: rogue/object.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/object.c,v retrieving revision 1.5 diff -u -r1.5 object.c --- rogue/object.c 30 Nov 1999 03:49:25 -0000 1.5 +++ rogue/object.c 21 Aug 2003 20:04:13 -0000 @@ -159,7 +159,6 @@ extern short cur_level, max_level; extern short party_room; -extern char *error_file; extern boolean is_wood[]; put_objects() Index: rogue/pack.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/pack.c,v retrieving revision 1.8 diff -u -r1.8 pack.c --- rogue/pack.c 30 Nov 1999 03:49:25 -0000 1.8 +++ rogue/pack.c 21 Aug 2003 20:07:28 -0000 @@ -342,7 +342,7 @@ char desc[DCOLS]; if (rogue.armor) { - message("your already wearing some", 0); + message("you're already wearing some", 0); return; } ch = pack_letter("wear what?", ARMOR); Index: rogue/rogue.h =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/rogue.h,v retrieving revision 1.3.2.1 diff -u -r1.3.2.1 rogue.h --- rogue/rogue.h 17 Dec 2001 12:43:23 -0000 1.3.2.1 +++ rogue/rogue.h 21 Aug 2003 21:11:28 -0000 @@ -194,9 +194,12 @@ #define MAX_OPT_LEN 40 +#define HUNGER_STR_LEN 8 + +#define MAX_ID_TITLE_LEN 64 struct id { short value; - char *title; + char title[MAX_ID_TITLE_LEN]; char *real; unsigned short id_status; }; @@ -472,3 +475,9 @@ short second; /* 0 - 59 */ }; +/* + * external variable declarations. + */ +extern char hunger_str[HUNGER_STR_LEN]; +extern char login_name[MAX_OPT_LEN]; +extern const char *error_file; Index: rogue/save.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/save.c,v retrieving revision 1.6 diff -u -r1.6 save.c --- rogue/save.c 30 Nov 1999 03:49:27 -0000 1.6 +++ rogue/save.c 21 Aug 2003 20:03:37 -0000 @@ -63,8 +63,6 @@ extern boolean detect_monster; extern short cur_level, max_level; -extern char hunger_str[]; -extern char login_name[]; extern short party_room; extern short foods; extern boolean is_wood[]; @@ -102,15 +100,23 @@ { FILE *fp; int file_id; - char name_buffer[80]; + char *name_buffer; + size_t len; char *hptr; struct rogue_time rt_buf; if (sfile[0] == '~') { if (hptr = md_getenv("HOME")) { - (void) strcpy(name_buffer, hptr); - (void) strcat(name_buffer, sfile+1); - sfile = name_buffer; + len = strlen(hptr) + strlen(sfile); + name_buffer = md_malloc(len); + if (name_buffer == NULL) { + message("out of memory for save file name", 0); + sfile = error_file; + } else { + (void) strcpy(name_buffer, hptr); + (void) strcat(name_buffer, sfile+1); + sfile = name_buffer; + } } } /* revoke */ @@ -199,10 +205,10 @@ r_read(fp, (char *) &detect_monster, sizeof(detect_monster)); r_read(fp, (char *) &cur_level, sizeof(cur_level)); r_read(fp, (char *) &max_level, sizeof(max_level)); - read_string(hunger_str, fp); + read_string(hunger_str, fp, sizeof hunger_str); - (void) strcpy(tbuf, login_name); - read_string(login_name, fp); + (void) strlcpy(tbuf, login_name, sizeof tbuf); + read_string(login_name, fp, sizeof login_name); if (strcmp(tbuf, login_name)) { clean_up("you're not the original player"); } @@ -345,7 +351,7 @@ r_read(fp, (char *) &(id_table[i].value), sizeof(short)); r_read(fp, (char *) &(id_table[i].id_status), sizeof(unsigned short)); - read_string(id_table[i].title, fp); + read_string(id_table[i].title, fp, MAX_ID_TITLE_LEN); } } } @@ -362,13 +368,16 @@ r_write(fp, s, n); } -read_string(s, fp) +read_string(s, fp, len) char *s; FILE *fp; +size_t len; { short n; r_read(fp, (char *) &n, sizeof(short)); + if (n > len) + clean_up("read_string: corrupt game file"); r_read(fp, s, n); xxxx(s, n); } Index: rogue/score.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/score.c,v retrieving revision 1.4 diff -u -r1.4 score.c --- rogue/score.c 30 Nov 1999 03:49:27 -0000 1.4 +++ rogue/score.c 21 Aug 2003 20:03:30 -0000 @@ -58,7 +58,6 @@ #include "rogue.h" #include "pathnames.h" -extern char login_name[]; extern char *m_names[]; extern short max_level; extern boolean score_only, no_skull, msg_cleared; Index: rogue/use.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/use.c,v retrieving revision 1.4 diff -u -r1.4 use.c --- rogue/use.c 30 Nov 1999 03:49:29 -0000 1.4 +++ rogue/use.c 21 Aug 2003 20:06:00 -0000 @@ -68,7 +68,6 @@ const char *strange_feeling = "you have a strange feeling for a moment, then it passes"; extern short bear_trap; -extern char hunger_str[]; extern short cur_room; extern long level_points[]; extern boolean being_held; --nFreZHaLTZJo0R7j--