From owner-freebsd-audit Sun Aug 6 22:32:25 2000 Delivered-To: freebsd-audit@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (Postfix) with ESMTP id 78AF037B709; Sun, 6 Aug 2000 22:32:22 -0700 (PDT) (envelope-from kris@FreeBSD.org) Received: from localhost (kris@localhost) by freefall.freebsd.org (8.9.3/8.9.2) with ESMTP id WAA08658; Sun, 6 Aug 2000 22:32:22 -0700 (PDT) (envelope-from kris@FreeBSD.org) X-Authentication-Warning: freefall.freebsd.org: kris owned process doing -bs Date: Sun, 6 Aug 2000 22:32:22 -0700 (PDT) From: Kris Kennaway To: Mike Heffner Cc: Kris Kennaway , audit@freebsd.org Subject: RE: catopen() patch In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sat, 5 Aug 2000, Mike Heffner wrote: > We can still walk right off the end. Right you are! This isn't such a big deal since $NLSPATH isn't read if we;re setugid, but how does this patch look (relative to the one I already committed): Index: msgcat.c =================================================================== RCS file: /home/ncvs/src/lib/libc/nls/msgcat.c,v retrieving revision 1.22 diff -u -r1.22 msgcat.c --- msgcat.c 2000/08/05 04:56:43 1.22 +++ msgcat.c 2000/08/07 05:28:46 @@ -124,13 +124,14 @@ strcpy(cptr, nlspath); cptr[len] = ':'; cptr[len+1] = '\0'; + spcleft = sizeof(path); for (nlspath = cptr; *cptr; ++cptr) { if (*cptr == ':') { *cptr = '\0'; - for (pathP = path; *nlspath; ++nlspath) { + for (pathP = path; *nlspath && spcleft > 0; ++nlspath) { + spcleft = sizeof(path) - (pathP - path); if (*nlspath == '%') { - spcleft = sizeof(path) - (pathP - path); if (*(nlspath + 1) == 'L') { ++nlspath; if (strlcpy(pathP, lang, spcleft) >= spcleft) { Kris -- In God we Trust -- all others must submit an X.509 certificate. -- Charles Forsythe To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Mon Aug 7 15:56:14 2000 Delivered-To: freebsd-audit@freebsd.org Received: from smtp1a.ispchannel.com (smtp.ispchannel.com [24.142.63.7]) by hub.freebsd.org (Postfix) with ESMTP id 6119837BB62; Mon, 7 Aug 2000 15:56:11 -0700 (PDT) (envelope-from mheffner@mailandnews.com) Received: from muriel.penguinpowered.com ([208.138.198.109]) by smtp1a.ispchannel.com (InterMail vK.4.02.00.00 201-232-116 license 7d3764cdaca754bf8ae20adf0db2aa60) with ESMTP id <20000807225757.CMKF8223.smtp1a@muriel.penguinpowered.com>; Mon, 7 Aug 2000 15:57:57 -0700 Content-Length: 2246 Message-ID: X-Mailer: XFMail 1.4.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: Date: Mon, 07 Aug 2000 18:55:03 -0400 (EDT) Reply-To: Mike Heffner From: Mike Heffner To: Kris Kennaway Subject: RE: catopen() patch Cc: audit@freebsd.org Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Well, I think we need to calculate spcleft at the end of each iteration, and there needs to be one char left for the NULL. How does this patch look (against rev. 1.22): --- msgcat.c.orig Mon Aug 7 18:42:28 2000 +++ msgcat.c Mon Aug 7 18:51:42 2000 @@ -128,9 +128,10 @@ for (nlspath = cptr; *cptr; ++cptr) { if (*cptr == ':') { *cptr = '\0'; - for (pathP = path; *nlspath; ++nlspath) { + for (pathP = path, spcleft = sizeof(path); + *nlspath && spcleft > 1; + ++nlspath, spcleft = sizeof(path) - (pathP - path)) { if (*nlspath == '%') { - spcleft = sizeof(path) - (pathP - path); if (*(nlspath + 1) == 'L') { ++nlspath; if (strlcpy(pathP, lang, spcleft) >= spcleft) { On 07-Aug-2000 Kris Kennaway wrote: | | Right you are! This isn't such a big deal since $NLSPATH isn't read if | we;re setugid, but how does this patch look (relative to the one I | already committed): | | Index: msgcat.c | =================================================================== | RCS file: /home/ncvs/src/lib/libc/nls/msgcat.c,v | retrieving revision 1.22 | diff -u -r1.22 msgcat.c | --- msgcat.c 2000/08/05 04:56:43 1.22 | +++ msgcat.c 2000/08/07 05:28:46 | @@ -124,13 +124,14 @@ | strcpy(cptr, nlspath); | cptr[len] = ':'; | cptr[len+1] = '\0'; | + spcleft = sizeof(path); | | for (nlspath = cptr; *cptr; ++cptr) { | if (*cptr == ':') { | *cptr = '\0'; | - for (pathP = path; *nlspath; ++nlspath) { | + for (pathP = path; *nlspath && spcleft > 0; ++nlspath) { | + spcleft = sizeof(path) - (pathP - path); | if (*nlspath == '%') { | - spcleft = sizeof(path) - (pathP - path); | if (*(nlspath + 1) == 'L') { | ++nlspath; | if (strlcpy(pathP, lang, spcleft) >= spcleft) { | -- Mike Heffner Fredericksburg, VA ICQ# 882073 http://my.ispchannel.com/~mheffner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Aug 9 0:16:18 2000 Delivered-To: freebsd-audit@freebsd.org Received: from gateway.posi.net (c1096725-a.smateo1.sfba.home.com [24.20.139.104]) by hub.freebsd.org (Postfix) with ESMTP id 1A82E37BDCA; Wed, 9 Aug 2000 00:16:15 -0700 (PDT) (envelope-from kbyanc@posi.net) Received: from localhost (kbyanc@localhost) by gateway.posi.net (8.9.3/8.9.3) with ESMTP id AAA19394; Wed, 9 Aug 2000 00:20:12 -0700 (PDT) (envelope-from kbyanc@posi.net) Date: Wed, 9 Aug 2000 00:20:11 -0700 (PDT) From: Kelly Yancey To: Kris Kennaway Cc: audit@FreeBSD.ORG Subject: Re: Update to patch(1) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Tue, 1 Aug 2000, Kris Kennaway wrote: > On Sun, 2 Jul 2000, Kelly Yancey wrote: > > > > > Can someone please review the patches in PR 19642. They merge in many > > changes to patch(1) from OpenBSD. Specifically, they remove the standard > > mktemp race condition as well as fix some potential buffer overflows. > > Sorry for the delay. > > Some comments: > > * be consistent about sizeof(foo) vs sizeof foo (choose whichever the > surrounding file uses) > Yeah, unfortunatly the surrounding files aren't consistent either. :( The existing code flips back and forth between the two at will. > * system() is insecure - there's no point in making all the string > operations buffer-safe if you go and pass a user string to system() :-) > I can only assume that the original OpenBSD patches were more for consistency's sake. It can't hurt, though. :) > * mkstemp() + close() isn't a drop-in replacement for mktemp() since it > will leave tempfiles around if the program exits through an abnormal > channel (error condition, signal, etc). mkstemp() + unlink() is usually > okay if the program (or another program) doesn't need to reopen the same > file, although it needs more source-code modification. > Hmm. That is a good point and an interesting dilemma: without making some fairly intruisive changes I can't use mkstemp() + unlink(). So which is the lesser evil: the existing use of mktemp or risking leaving tempfiles with mkstemp()? > Kris > Thanks for reviewing these and I look forward to your comments. Kelly -- Kelly Yancey - kbyanc@posi.net - Belmont, CA System Administrator, eGroups.com http://www.egroups.com/ Maintainer, BSD Driver Database http://www.posi.net/freebsd/drivers/ Coordinator, Team FreeBSD http://www.posi.net/freebsd/Team-FreeBSD/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Aug 9 0:21:29 2000 Delivered-To: freebsd-audit@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (Postfix) with ESMTP id 9543637B950; Wed, 9 Aug 2000 00:21:27 -0700 (PDT) (envelope-from kris@FreeBSD.org) Received: from localhost (kris@localhost) by freefall.freebsd.org (8.9.3/8.9.2) with ESMTP id AAA93070; Wed, 9 Aug 2000 00:21:27 -0700 (PDT) (envelope-from kris@FreeBSD.org) X-Authentication-Warning: freefall.freebsd.org: kris owned process doing -bs Date: Wed, 9 Aug 2000 00:21:27 -0700 (PDT) From: Kris Kennaway To: Kelly Yancey Cc: audit@FreeBSD.ORG Subject: Re: Update to patch(1) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Wed, 9 Aug 2000, Kelly Yancey wrote: > Yeah, unfortunatly the surrounding files aren't consistent either. :( The > existing code flips back and forth between the two at will. Hmm..it still would be good to choose a consistent style yourself, I guess. > > * system() is insecure - there's no point in making all the string > > operations buffer-safe if you go and pass a user string to system() :-) > > I can only assume that the original OpenBSD patches were more for > consistency's sake. It can't hurt, though. :) Well, there's more to auditing than just making things buffer-safe, although sometimes the other problems are overlooked. I can only assume that happened here.. > > * mkstemp() + close() isn't a drop-in replacement for mktemp() since it > > will leave tempfiles around if the program exits through an abnormal > > channel (error condition, signal, etc). mkstemp() + unlink() is usually > > okay if the program (or another program) doesn't need to reopen the same > > file, although it needs more source-code modification. > > > > Hmm. That is a good point and an interesting dilemma: without making some > fairly intruisive changes I can't use mkstemp() + unlink(). So which is the > lesser evil: the existing use of mktemp or risking leaving tempfiles with > mkstemp()? Could you do something evil like making a global variable for the file descriptor so you don't have to pass it around through function calls? Kris -- In God we Trust -- all others must submit an X.509 certificate. -- Charles Forsythe To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Aug 10 13:18:17 2000 Delivered-To: freebsd-audit@freebsd.org Received: from gateway.posi.net (c1096725-a.smateo1.sfba.home.com [24.20.139.104]) by hub.freebsd.org (Postfix) with ESMTP id 8D89337BCB5; Thu, 10 Aug 2000 13:18:06 -0700 (PDT) (envelope-from kbyanc@posi.net) Received: from localhost (kbyanc@localhost) by gateway.posi.net (8.9.3/8.9.3) with ESMTP id NAA24093; Thu, 10 Aug 2000 13:22:49 -0700 (PDT) (envelope-from kbyanc@posi.net) Date: Thu, 10 Aug 2000 13:22:49 -0700 (PDT) From: Kelly Yancey To: Kris Kennaway Cc: audit@FreeBSD.org Subject: Re: Update to patch(1) In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Wed, 9 Aug 2000, Kris Kennaway wrote: > > > * system() is insecure - there's no point in making all the string > > > operations buffer-safe if you go and pass a user string to system() :-) > > > > I can only assume that the original OpenBSD patches were more for > > consistency's sake. It can't hurt, though. :) > > Well, there's more to auditing than just making things buffer-safe, > although sometimes the other problems are overlooked. I can only assume > that happened here.. > No problem. How does a fork/exec combo sound to you, instead. That way it never gets to the shell. I don't see any other alternative. > > Hmm. That is a good point and an interesting dilemma: without making some > > fairly intruisive changes I can't use mkstemp() + unlink(). So which is the > > lesser evil: the existing use of mktemp or risking leaving tempfiles with > > mkstemp()? > > Could you do something evil like making a global variable for the file > descriptor so you don't have to pass it around through function calls? > Yeah, that fell into my definition of intrusive (a number of functions take pathnames as arguments which will have to go awayor be replaced with descriptor arguments). Frankly, that is fine with me, I was more concerned that someone might object to making such major modifications to a contrib'ed source (albeit only technically contrib'ed). But if there is no objection, I'll take the axe to it. :) Kelly -- Kelly Yancey - kbyanc@posi.net - Belmont, CA System Administrator, eGroups.com http://www.egroups.com/ Maintainer, BSD Driver Database http://www.posi.net/freebsd/drivers/ Coordinator, Team FreeBSD http://www.posi.net/freebsd/Team-FreeBSD/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Fri Aug 11 22:33:22 2000 Delivered-To: freebsd-audit@freebsd.org Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 0CFDA37BB01; Fri, 11 Aug 2000 22:32:57 -0700 (PDT) (envelope-from green@FreeBSD.org) Date: Sat, 12 Aug 2000 01:32:38 -0400 (EDT) From: Brian Fundakowski Feldman X-Sender: green@green.dyndns.org To: Kris Kennaway Cc: audit@freebsd.org Subject: Re: Fuzz testing In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Mon, 31 Jul 2000, Kris Kennaway wrote: > For example: > > a2p.core as.core csh.core flex++.core flex.core sh.core I've been tracking down sh.core, because I consider this very important. The shells _must_ be secure, and "crashing" bugs certainly don't make them seem so. In the sh(1) case, it crashes on input of control characters. This wouldn't be a problem normally, because there is tons of code in sh(1) that is made to support escaping all evil control characters in the input. However, Martin Cracauer seems to think making it 8-bit clean is done by not escaping the control characters :-( I have no idea how you would believe that control characters are "okay" to leave unescaped "just because" they're used by a character set, and indeed that should be all the more reason to make sure they're properly escaped. This needs a hell of a lot of reversion to fix. Yes, I think this probably security implications :-( -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / green@FreeBSD.org `------------------------------' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Aug 12 5:48:32 2000 Delivered-To: freebsd-audit@freebsd.org Received: from knight.cons.org (knight.cons.org [194.233.237.86]) by hub.freebsd.org (Postfix) with ESMTP id E88C537BD1A; Sat, 12 Aug 2000 05:48:28 -0700 (PDT) (envelope-from cracauer@knight.cons.org) Received: (from cracauer@localhost) by knight.cons.org (8.9.3/8.9.3) id OAA03215; Sat, 12 Aug 2000 14:48:23 +0200 (CEST) Date: Sat, 12 Aug 2000 14:48:23 +0200 From: Martin Cracauer To: Brian Fundakowski Feldman Cc: Kris Kennaway , audit@FreeBSD.ORG Subject: Re: Fuzz testing Message-ID: <20000812144822.A3193@cons.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 1.0.1i In-Reply-To: ; from green@FreeBSD.ORG on Sat, Aug 12, 2000 at 01:32:38AM -0400 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG In , Brian Fundakowski Feldman wrote: > On Mon, 31 Jul 2000, Kris Kennaway wrote: > > > For example: > > > > a2p.core as.core csh.core flex++.core flex.core sh.core > > I've been tracking down sh.core, because I consider this very > important. The shells _must_ be secure, and "crashing" bugs certainly > don't make them seem so. In the sh(1) case, it crashes on input of > control characters. This wouldn't be a problem normally, because > there is tons of code in sh(1) that is made to support escaping all > evil control characters in the input. > > However, Martin Cracauer seems to think making it 8-bit clean is done > by not escaping the control characters :-( I have no idea how you > would believe that control characters are "okay" to leave unescaped > "just because" they're used by a character set, and indeed that should > be all the more reason to make sure they're properly escaped. This is FUD, . I said it is preferrable to change the whole stuff to 16 bits per char (different space for chars and control things) over just escaping chars in the same space. Never did I neglect the problem. 16-bits/different space is certainly more secure since you are robust against coding errors in all the zillion place you would need to take escaped chars into account. Just never have a control thing where you would expect a char (in the lower 8 bits of the 16). > This needs a hell of a lot of reversion to fix. Yes, I think this > probably security implications :-( The complex sh rules make it almost impossible to make sh scripts secure, no matter how good the implementation is. Anyone who exectutes possibly unfriendly sh scripts under a to-be-protected userid is just insane. Again, I certainly will make sh 8-bit clean unless someone pisses me off too badly. Martin -- %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% Martin Cracauer http://www.cons.org/cracauer/ BSD User Group Hamburg, Germany http://www.bsdhh.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Aug 12 8:12:52 2000 Delivered-To: freebsd-audit@freebsd.org Received: from localhost (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id ACE9037B6CB; Sat, 12 Aug 2000 08:12:46 -0700 (PDT) (envelope-from green@FreeBSD.org) Date: Sat, 12 Aug 2000 11:12:45 -0400 (EDT) From: Brian Fundakowski Feldman X-Sender: green@green.dyndns.org To: Martin Cracauer Cc: Kris Kennaway , audit@FreeBSD.ORG Subject: Re: Fuzz testing In-Reply-To: <20000812144822.A3193@cons.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sat, 12 Aug 2000, Martin Cracauer wrote: > > However, Martin Cracauer seems to think making it 8-bit clean is done > > by not escaping the control characters :-( I have no idea how you > > would believe that control characters are "okay" to leave unescaped > > "just because" they're used by a character set, and indeed that should > > be all the more reason to make sure they're properly escaped. > > This is FUD, . If I'm pissed off enough about the problem to make it seem like that, I'm glad. Call me what you like. > I said it is preferrable to change the whole stuff to 16 bits per char > (different space for chars and control things) over just escaping > chars in the same space. Never did I neglect the problem. Of course that's preferable, but can you tell me what deltas like this are doing: @@ -906,7 +908,13 @@ CHECKEND(); /* set c to PEOF if at end of here document */ for (;;) { /* until end of line or end of word */ CHECKSTRSPACE(3, out); /* permit 3 calls to USTPUTC */ - switch(syntax[c]) { + + if (c < 0 && c != PEOF) + synentry = CWORD; + else + synentry = syntax[c]; + + switch(synentry) { case CNL: /* '\n' */ if (syntax == BASESYNTAX) goto endword; /* exit outer loop */ How in the world is is "putting your finger in your ears" about control characters better than escaping them?? > 16-bits/different space is certainly more secure since you are robust > against coding errors in all the zillion place you would need to take > escaped chars into account. Just never have a control thing where you > would expect a char (in the lower 8 bits of the 16). What justification is there to remove the carefully coded control escapes that have been there before? > > This needs a hell of a lot of reversion to fix. Yes, I think this > > probably security implications :-( > > The complex sh rules make it almost impossible to make sh scripts > secure, no matter how good the implementation is. Anyone who > exectutes possibly unfriendly sh scripts under a to-be-protected > userid is just insane. I'm not talking about possibly unfriendly. I'm talking about your own. Am I to understand that none of the control sequence parsing code ever deals with user input? > Again, I certainly will make sh 8-bit clean unless someone pisses me > off too badly. So how is it more clean here: {"/home/green"}$ printf echo\ '\204'Hello. | sh Memory fault (core dumped) > Martin > -- > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% > Martin Cracauer http://www.cons.org/cracauer/ > BSD User Group Hamburg, Germany http://www.bsdhh.org/ > -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / green@FreeBSD.org `------------------------------' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message