From owner-freebsd-audit Sun Dec 17 9:23:25 2000 From owner-freebsd-audit@FreeBSD.ORG Sun Dec 17 09:23:23 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from genius.tao.org.uk (genesis.tao.org.uk [194.242.131.94]) by hub.freebsd.org (Postfix) with ESMTP id E042737B400 for ; Sun, 17 Dec 2000 09:23:22 -0800 (PST) Received: by genius.tao.org.uk (Postfix, from userid 100) id E7A243137; Sun, 17 Dec 2000 17:23:21 +0000 (GMT) Date: Sun, 17 Dec 2000 17:23:21 +0000 From: Josef Karthauser To: Chris Faulhaber Cc: freebsd-audit@FreeBSD.ORG Subject: Re: crunchgen(8) patch (again) Message-ID: <20001217172321.E1428@tao.org.uk> References: <20001203072512.A86744@earth.causticlabs.com> <20001210124833.B84921@bsdi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20001210124833.B84921@bsdi.com>; from joe@tao.org.uk on Sun, Dec 10, 2000 at 12:48:33PM +0000 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sun, Dec 10, 2000 at 12:48:33PM +0000, Josef Karthauser wrote: > I'll take this one - I've got local patches to crunchgen in my > tree that I'm going to commit pending review on freebsd-small. > After that I'm going to style(9) it and will incorporate these > patches during that process. I've committed this - will MFC later. Joe > On Sun, Dec 03, 2000 at 07:25:12AM -0500, Chris Faulhaber wrote: > > The following patch fixes: > > > > o check strdup() return values > > o strcpy() -> strlcpy() > > o sprintf() -> snprintf() > > o mktemp() -> mkstemp() > > o use err() instead of errx() in out_of_memory() function since > > errno will probably be set > > > > Also, I have quite a few small patches for review at: > > http://www.fxp.org/~jedgar/FreeBSD/diffs/ > > > > -- > > Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org > > -------------------------------------------------------- > > FreeBSD: The Power To Serve - http://www.FreeBSD.org > > -- > Josef Karthauser [joe@FreeBSD.org, joe@tao.org.uk] > ........ FreeBSD: The power to change the world ........ > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-audit" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Dec 19 6:35:11 2000 From owner-freebsd-audit@FreeBSD.ORG Tue Dec 19 06:35:09 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from scientia.demon.co.uk (scientia.demon.co.uk [212.228.14.13]) by hub.freebsd.org (Postfix) with ESMTP id 8578337B400 for ; Tue, 19 Dec 2000 06:35:08 -0800 (PST) Received: from strontium.scientia.demon.co.uk ([192.168.91.36] ident=root) by scientia.demon.co.uk with esmtp (Exim 3.169 #1) id 148Nr9-0004HL-00 for audit@FreeBSD.org; Tue, 19 Dec 2000 14:35:07 +0000 Received: (from ben@localhost) by strontium.scientia.demon.co.uk (8.11.1/8.11.1) id eBJEZ7748511 for audit@FreeBSD.org; Tue, 19 Dec 2000 14:35:07 GMT (envelope-from ben) Date: Tue, 19 Dec 2000 14:35:06 +0000 From: Ben Smithurst To: audit@FreeBSD.org Subject: printf(1) broken for some long format strings Message-ID: <20001219143506.C78749@strontium.scientia.demon.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: ben@scientia.demon.co.uk Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG [previously posted to -developers; posted to -audit too at Will Andrews' suggestion.] printf(1) is broken for some long format strings, like printf "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX%d\n" 1 for a sufficiently large number of X's. Does anyone have any objections to this fix? thanks. Index: printf.c =================================================================== RCS file: /usr/cvs/src/usr.bin/printf/printf.c,v retrieving revision 1.15 diff -u -r1.15 printf.c --- printf.c 2000/09/04 06:11:25 1.15 +++ printf.c 2000/12/19 02:17:18 @@ -247,12 +247,18 @@ char *str; int ch; { - static char copy[64]; + static char *copy = NULL; + static size_t copy_size = 0; + char *newcopy; int len; len = strlen(str) + 2; - if (len > sizeof copy) - return NULL; + if (len > copy_size) { + if ((newcopy = realloc(copy, len)) == NULL) + return NULL; + copy = newcopy; + copy_size = len; + } memmove(copy, str, len - 3); copy[len - 3] = 'q'; -- Ben Smithurst / ben@FreeBSD.org / PGP: 0x99392F7D To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Dec 19 9:29:26 2000 From owner-freebsd-audit@FreeBSD.ORG Tue Dec 19 09:29:24 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from rover.village.org (rover.village.org [204.144.255.66]) by hub.freebsd.org (Postfix) with ESMTP id 3FD6937B400; Tue, 19 Dec 2000 09:29:23 -0800 (PST) Received: from billy-club.village.org (billy-club.village.org [10.0.0.3]) by rover.village.org (8.11.0/8.11.0) with ESMTP id eBJHTKs13933; Tue, 19 Dec 2000 10:29:21 -0700 (MST) (envelope-from imp@billy-club.village.org) Received: from billy-club.village.org (localhost [127.0.0.1]) by billy-club.village.org (8.11.1/8.8.3) with ESMTP id eBJHTps36903; Tue, 19 Dec 2000 10:29:51 -0700 (MST) Message-Id: <200012191729.eBJHTps36903@billy-club.village.org> To: Ben Smithurst Subject: Re: printf(1) broken for some long format strings Cc: audit@FreeBSD.ORG In-reply-to: Your message of "Tue, 19 Dec 2000 14:35:06 GMT." <20001219143506.C78749@strontium.scientia.demon.co.uk> References: <20001219143506.C78749@strontium.scientia.demon.co.uk> Date: Tue, 19 Dec 2000 10:29:51 -0700 From: Warner Losh Sender: imp@billy-club.village.org Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG In message <20001219143506.C78749@strontium.scientia.demon.co.uk> Ben Smithurst writes: : + if (len > copy_size) { : + if ((newcopy = realloc(copy, len)) == NULL) : + return NULL; : + copy = newcopy; : + copy_size = len; : + } if (len > copy_size) { if (copy_size == 0) newlen = 1024; else newlen = ((len + 1023) >> 10) << 10; if ((newcopy = realloc(copy, newlen)) == NULL) return NULL; copy = newcopy; copy_size = newlen; } might be a little better. It will round the size to the next highest k boundary, which will result in fewer malloc calls. A slightly better way would be to find some way to round it up to the next power of 2 so that you do even fewer malloc calls, but my bit tiddling foo isn't awake yet this morning. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Dec 19 13:46: 2 2000 From owner-freebsd-audit@FreeBSD.ORG Tue Dec 19 13:46:00 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from scientia.demon.co.uk (scientia.demon.co.uk [212.228.14.13]) by hub.freebsd.org (Postfix) with ESMTP id B336A37B404 for ; Tue, 19 Dec 2000 13:45:51 -0800 (PST) Received: from strontium.scientia.demon.co.uk ([192.168.91.36] ident=root) by scientia.demon.co.uk with esmtp (Exim 3.169 #1) id 148ODO-0004Le-00 for audit@FreeBSD.org; Tue, 19 Dec 2000 14:58:06 +0000 Received: (from ben@localhost) by strontium.scientia.demon.co.uk (8.11.1/8.11.1) id eBJEw6H34641 for audit@FreeBSD.org; Tue, 19 Dec 2000 14:58:06 GMT (envelope-from ben) Date: Tue, 19 Dec 2000 14:58:06 +0000 From: Ben Smithurst To: audit@FreeBSD.org Subject: Re: printf(1) broken for some long format strings Message-ID: <20001219145806.F78749@strontium.scientia.demon.co.uk> References: <20001219143506.C78749@strontium.scientia.demon.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20001219143506.C78749@strontium.scientia.demon.co.uk> Sender: ben@scientia.demon.co.uk Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG > [previously posted to -developers; posted to -audit too at Will Andrews' > suggestion.] I've made some changes based on some comments from bde... I think this addresses all of the points he made. (I don't read -audit so please remember to CC any comments to me, thanks.) Index: printf.c =================================================================== RCS file: /usr/cvs/src/usr.bin/printf/printf.c,v retrieving revision 1.15 diff -u -r1.15 printf.c --- printf.c 2000/09/04 06:11:25 1.15 +++ printf.c 2000/12/19 14:53:31 @@ -60,6 +60,7 @@ #ifdef SHELL #define main printfcmd #include "bltin/bltin.h" +#include "memalloc.h" #else #define warnx1(a, b, c) warnx(a) #define warnx2(a, b, c) warnx(a, b) @@ -247,12 +248,25 @@ char *str; int ch; { - static char copy[64]; - int len; + static char *copy; + static size_t copy_size; + size_t len; len = strlen(str) + 2; - if (len > sizeof copy) - return NULL; + if (len > copy_size) { +#ifdef SHELL + char *newcopy; + if ((newcopy = ckrealloc(copy, len)) == NULL) + return (NULL); + copy = newcopy; +#else + if ((copy = reallocf(copy, len)) == NULL) { + copy_size = 0; + return (NULL); + } +#endif + copy_size = len; + } memmove(copy, str, len - 3); copy[len - 3] = 'q'; -- Ben Smithurst / ben@FreeBSD.org / PGP: 0x99392F7D To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Dec 19 13:54:46 2000 From owner-freebsd-audit@FreeBSD.ORG Tue Dec 19 13:54:40 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from scientia.demon.co.uk (scientia.demon.co.uk [212.228.14.13]) by hub.freebsd.org (Postfix) with ESMTP id 843D037B400 for ; Tue, 19 Dec 2000 13:54:31 -0800 (PST) Received: from strontium.scientia.demon.co.uk ([192.168.91.36] ident=root) by scientia.demon.co.uk with esmtp (Exim 3.169 #1) id 148UiG-0004oZ-00; Tue, 19 Dec 2000 21:54:24 +0000 Received: (from ben@localhost) by strontium.scientia.demon.co.uk (8.11.1/8.11.1) id eBJLsEY28134; Tue, 19 Dec 2000 21:54:14 GMT (envelope-from ben) Date: Tue, 19 Dec 2000 21:54:14 +0000 From: Ben Smithurst To: Warner Losh Cc: audit@FreeBSD.ORG Subject: Re: printf(1) broken for some long format strings Message-ID: <20001219215413.H78749@strontium.scientia.demon.co.uk> References: <20001219143506.C78749@strontium.scientia.demon.co.uk> <200012191729.eBJHTps36903@billy-club.village.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200012191729.eBJHTps36903@billy-club.village.org> Sender: ben@scientia.demon.co.uk Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Warner Losh wrote: > if (len > copy_size) { > if (copy_size == 0) > newlen = 1024; what's the point of the special case for copy_size == 0? And that would break if more than 1k is needed the first time through, wouldn't it? (Or maybe I'm just not thinking straight at the moment.) Why not just use > newlen = ((len + 1023) >> 10) << 10; every time? > might be a little better. It will round the size to the next highest > k boundary, which will result in fewer malloc calls. A slightly > better way would be to find some way to round it up to the next power > of 2 so that you do even fewer malloc calls, but my bit tiddling foo > isn't awake yet this morning. that's probably over the top, since the maximum length before this change was 64 bytes, there can't be many places where more than that is needed or someone would have noticed this and fixed it before me. :-) -- Ben Smithurst / ben@FreeBSD.org / PGP: 0x99392F7D To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Dec 19 17:42:43 2000 From owner-freebsd-audit@FreeBSD.ORG Tue Dec 19 17:42:41 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from netau1.alcanet.com.au (ntp.alcanet.com.au [203.62.196.27]) by hub.freebsd.org (Postfix) with ESMTP id D930B37B404; Tue, 19 Dec 2000 17:42:39 -0800 (PST) Received: from mfg1.cim.alcatel.com.au (mfg1.cim.alcatel.com.au [139.188.23.1]) by netau1.alcanet.com.au (8.9.3 (PHNE_18979)/8.9.3) with ESMTP id MAA06496; Wed, 20 Dec 2000 12:42:36 +1100 (EDT) Received: from gsmx07.alcatel.com.au by cim.alcatel.com.au (PMDF V5.2-32 #37640) with ESMTP id <01JXXEOQZE0G902KLJ@cim.alcatel.com.au>; Wed, 20 Dec 2000 12:42:34 +1100 Received: (from jeremyp@localhost) by gsmx07.alcatel.com.au (8.11.0/8.11.0) id eBK1gWF63786; Wed, 20 Dec 2000 12:42:32 +1100 (EST envelope-from jeremyp) Content-return: prohibited Date: Wed, 20 Dec 2000 12:42:32 +1100 From: Peter Jeremy Subject: Re: printf(1) broken for some long format strings In-reply-to: <20001219215413.H78749@strontium.scientia.demon.co.uk>; from ben@FreeBSD.ORG on Tue, Dec 19, 2000 at 09:54:14PM +0000 To: Ben Smithurst Cc: audit@FreeBSD.ORG Mail-followup-to: Ben Smithurst , audit@FreeBSD.ORG Message-id: <20001220124232.I54775@gsmx07.alcatel.com.au> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-disposition: inline User-Agent: Mutt/1.2.5i References: <20001219143506.C78749@strontium.scientia.demon.co.uk> <200012191729.eBJHTps36903@billy-club.village.org> <20001219215413.H78749@strontium.scientia.demon.co.uk> Sender: jeremyp@gsmx07.alcatel.com.au Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On 2000-Dec-19 21:54:14 +0000, Ben Smithurst wrote: >Warner Losh wrote: >Why not just use > >> newlen = ((len + 1023) >> 10) << 10; > >every time? That would seem the better solution. >> might be a little better. It will round the size to the next highest >> k boundary, which will result in fewer malloc calls. > >that's probably over the top, since the maximum length before this >change was 64 bytes, there can't be many places where more than that is >needed or someone would have noticed this and fixed it before me. :-) One problem with your patch is that it just ensures that there is sufficient space for the current copy. This means that realloc() is likely to be called a number of times with slightly larger lengths each time. This sort of behaviour is likely to lead to memory fragmentation, which may be significant in long running processes (like interactive shells). Over-estimating the amount of memory needed when actually allocating memory will reduce the number of realloc()s and hence the amount of fragmentation. Peter To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Dec 20 3:46:10 2000 From owner-freebsd-audit@FreeBSD.ORG Wed Dec 20 03:46:08 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from scientia.demon.co.uk (scientia.demon.co.uk [212.228.14.13]) by hub.freebsd.org (Postfix) with ESMTP id 8852C37B400 for ; Wed, 20 Dec 2000 03:46:06 -0800 (PST) Received: from strontium.scientia.demon.co.uk ([192.168.91.36] ident=root) by scientia.demon.co.uk with esmtp (Exim 3.169 #1) id 148guG-0005sC-00 for audit@FreeBSD.ORG; Wed, 20 Dec 2000 10:55:36 +0000 Received: (from ben@localhost) by strontium.scientia.demon.co.uk (8.11.1/8.11.1) id eBKAtaw50726 for audit@FreeBSD.ORG; Wed, 20 Dec 2000 10:55:36 GMT (envelope-from ben) Date: Wed, 20 Dec 2000 10:55:36 +0000 From: Ben Smithurst To: audit@FreeBSD.ORG Subject: Re: printf(1) broken for some long format strings Message-ID: <20001220105536.A8350@strontium.scientia.demon.co.uk> References: <20001219143506.C78749@strontium.scientia.demon.co.uk> <200012191729.eBJHTps36903@billy-club.village.org> <20001219215413.H78749@strontium.scientia.demon.co.uk> <20001220124232.I54775@gsmx07.alcatel.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20001220124232.I54775@gsmx07.alcatel.com.au> Sender: ben@scientia.demon.co.uk Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Peter Jeremy wrote: > One problem with your patch is that it just ensures that there is > sufficient space for the current copy. This means that realloc() is > likely to be called a number of times with slightly larger lengths > each time. This sort of behaviour is likely to lead to memory > fragmentation, which may be significant in long running processes > (like interactive shells). Over-estimating the amount of memory > needed when actually allocating memory will reduce the number of > realloc()s and hence the amount of fragmentation. ok, but I don't think realloc() will be needed many times if we just round to the nearest k boundary. I think the buffer in mklong() is only needed when there's a long string with no format strings in it followed by a format like "%d", and it's probably fairly rare for printf(1) to be used with a string like that over 1k in length. Unless anyone knows otherwise. So, my final proposal. :-) I stopped using reallocf() even though Bruce wanted it, because we use ckrealloc() in the shell, the overall code is actually a bit simpler without reallocf(). Index: printf.c =================================================================== RCS file: /usr/cvs/src/usr.bin/printf/printf.c,v retrieving revision 1.15 diff -u -r1.15 printf.c --- printf.c 2000/09/04 06:11:25 1.15 +++ printf.c 2000/12/19 22:15:31 @@ -60,6 +60,7 @@ #ifdef SHELL #define main printfcmd #include "bltin/bltin.h" +#include "memalloc.h" #else #define warnx1(a, b, c) warnx(a) #define warnx2(a, b, c) warnx(a, b) @@ -247,12 +248,23 @@ char *str; int ch; { - static char copy[64]; - int len; + static char *copy; + static size_t copy_size; + size_t len, newlen; + char *newcopy; len = strlen(str) + 2; - if (len > sizeof copy) - return NULL; + if (len > copy_size) { + newlen = ((len + 1023) >> 10) << 10; +#ifdef SHELL + if ((newcopy = ckrealloc(copy, newlen)) == NULL) +#else + if ((newcopy = realloc(copy, newlen)) == NULL) +#endif + return (NULL); + copy = newcopy; + copy_size = newlen; + } memmove(copy, str, len - 3); copy[len - 3] = 'q'; -- Ben Smithurst / ben@FreeBSD.org / PGP: 0x99392F7D To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Dec 20 18:13:18 2000 From owner-freebsd-audit@FreeBSD.ORG Wed Dec 20 18:13:16 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from relay.nuxi.com (nuxi.cs.ucdavis.edu [169.237.7.38]) by hub.freebsd.org (Postfix) with ESMTP id 9F68937B400 for ; Wed, 20 Dec 2000 18:13:16 -0800 (PST) Received: from dragon.nuxi.com (Ipittythefoolthattrustsident@trang.nuxi.com [209.152.133.57]) by relay.nuxi.com (8.9.3/8.9.3) with ESMTP id SAA16782 for ; Wed, 20 Dec 2000 18:13:16 -0800 (PST) (envelope-from obrien@NUXI.com) Received: (from obrien@localhost) by dragon.nuxi.com (8.11.1/8.11.1) id eBL2DFA12843 for audit@freebsd.org; Wed, 20 Dec 2000 18:13:15 -0800 (PST) (envelope-from obrien) Date: Wed, 20 Dec 2000 18:13:14 -0800 From: "David O'Brien" To: audit@freebsd.org Subject: Heads Up need to audit the NetBSD ftpd code Message-ID: <20001220181314.D12671@dragon.nuxi.com> Reply-To: audit@freebsd.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i X-Operating-System: FreeBSD 5.0-CURRENT Organization: The NUXI BSD group X-Pgp-Rsa-Fingerprint: B7 4D 3E E9 11 39 5F A3 90 76 5D 69 58 D9 98 7A X-Pgp-Rsa-Keyid: 1024/34F9F9D5 Sender: obrien@NUXI.com Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG I intend to to import the NetBSD ftpd code in the near-term future. It has much more functionality of our's -- on par of having the most useful features of wu-ftpd. And many feel it is a better code base with less bug (especially ones that can be security vulnerabilities). However, before I import it, Warner Losh thought it should be brought up for review. So please have at ftp://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/libexec/ftpd/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Dec 20 21: 5:47 2000 From owner-freebsd-audit@FreeBSD.ORG Wed Dec 20 21:05:46 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mail5.registeredsite.com (mail5.registeredsite.com [64.224.9.14]) by hub.freebsd.org (Postfix) with ESMTP id 32B5F37B400; Wed, 20 Dec 2000 21:03:03 -0800 (PST) Received: from mail.techfour.net ([209.35.6.184]) by mail5.registeredsite.com (8.11.1/8.11.1) with ESMTP id eBL532j28327; Thu, 21 Dec 2000 00:03:02 -0500 Received: from muriel.penguinpowered.com [208.138.197.178] by mail.techfour.net with ESMTP (SMTPD32-6.00) id AF0620A20106; Thu, 21 Dec 2000 00:03:02 -0500 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: <20001220181314.D12671@dragon.nuxi.com> Date: Thu, 21 Dec 2000 00:02:56 -0500 (EST) Reply-To: Mike Heffner Sender: spock@muriel.penguinpowered.com From: Mike Heffner To: "David O'Brien" Subject: RE: Heads Up need to audit the NetBSD ftpd code Cc: audit@freebsd.org Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG This is more a matter of consistency rather than a security consideration, but maybe the use of MAXHOSTNAMELEN+1 in extern.h should be changed to MAXHOSTNAMELEN. On 21-Dec-2000 David O'Brien wrote: | I intend to to import the NetBSD ftpd code in the near-term future. It | has much more functionality of our's -- on par of having the most useful | features of wu-ftpd. And many feel it is a better code base with less | bug (especially ones that can be security vulnerabilities). | | However, before I import it, Warner Losh thought it should be brought up | for review. | | So please have at | ftp://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/libexec/ftpd/ -- Mike Heffner Blacksburg, 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 Thu Dec 21 5:58:31 2000 From owner-freebsd-audit@FreeBSD.ORG Thu Dec 21 05:58:29 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from peitho.fxp.org (peitho.fxp.org [209.26.95.40]) by hub.freebsd.org (Postfix) with ESMTP id 6439937B400 for ; Thu, 21 Dec 2000 05:58:29 -0800 (PST) Received: from earth.causticlabs.com (oca-c1s4-12.mfi.net [209.26.94.151]) by peitho.fxp.org (Postfix) with ESMTP id 89AB11360E for ; Thu, 21 Dec 2000 08:58:27 -0500 (EST) Received: by earth.causticlabs.com (Postfix, from userid 1000) id 08B691F5B; Thu, 21 Dec 2000 08:58:24 -0500 (EST) Date: Thu, 21 Dec 2000 08:58:24 -0500 From: Chris Faulhaber To: audit@freebsd.org Subject: Re: Heads Up need to audit the NetBSD ftpd code Message-ID: <20001221085824.A19639@earth.causticlabs.com> References: <20001220181314.D12671@dragon.nuxi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20001220181314.D12671@dragon.nuxi.com>; from obrien@freebsd.org on Wed, Dec 20, 2000 at 06:13:14PM -0800 Sender: jedgar@earth.causticlabs.com Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Wed, Dec 20, 2000 at 06:13:14PM -0800, David O'Brien wrote: > I intend to to import the NetBSD ftpd code in the near-term future. It > has much more functionality of our's -- on par of having the most useful > features of wu-ftpd. And many feel it is a better code base with less > bug (especially ones that can be security vulnerabilities). > > However, before I import it, Warner Losh thought it should be brought up > for review. > > So please have at > ftp://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/libexec/ftpd/ > Am I too paranoid in requesting that STAT and SYST only be available to users who are logged in? -- Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org -------------------------------------------------------- FreeBSD: The Power To Serve - http://www.FreeBSD.org Index: ftpcmd.y =================================================================== RCS file: /cvsroot/basesrc/libexec/ftpd/ftpcmd.y,v retrieving revision 1.48.2.1 diff -u -r1.48.2.1 ftpcmd.y --- ftpcmd.y 2000/07/25 08:38:38 1.48.2.1 +++ ftpcmd.y 2000/12/21 13:25:32 @@ -736,7 +736,7 @@ } } - | SYST CRLF + | SYST check_login CRLF { if (EMPTYSTR(version)) reply(215, "UNIX Type: L%d", NBBY); @@ -753,7 +753,7 @@ free($4); } - | STAT CRLF + | STAT check_login CRLF { statcmd(); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Fri Dec 22 12:46: 6 2000 From owner-freebsd-audit@FreeBSD.ORG Fri Dec 22 12:46:01 2000 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from peitho.fxp.org (peitho.fxp.org [209.26.95.40]) by hub.freebsd.org (Postfix) with ESMTP id 2343537B400 for ; Fri, 22 Dec 2000 12:46:00 -0800 (PST) Received: by peitho.fxp.org (Postfix, from userid 1000) id B27E81360E; Fri, 22 Dec 2000 15:45:59 -0500 (EST) Date: Fri, 22 Dec 2000 15:45:59 -0500 From: Chris Faulhaber To: freebsd-audit@FreeBSD.org Subject: openssh patch Message-ID: <20001222154559.A25487@peitho.fxp.org> Mail-Followup-To: Chris Faulhaber , freebsd-audit@FreeBSD.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Would anyone care to give the following patch a sanity-check before I submit it to the openssh folks? OpenSSH hardcodes /tmp in a few places for the creation of a directory to hold the agent's unix-domain socket (those /tmp/ssh_XXXXXXXXXX/ dirs that occasionally get left behind). I have added the option of allowing the TMPDIR env variable to override _PATH_TMP. -- Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org -------------------------------------------------------- FreeBSD: The Power To Serve - http://www.FreeBSD.org Index: channels.c =================================================================== RCS file: /home/ncvs/src/crypto/openssh/channels.c,v retrieving revision 1.1.1.1.2.2 diff -u -r1.1.1.1.2.2 channels.c --- channels.c 2000/10/28 23:00:47 1.1.1.1.2.2 +++ channels.c 2000/12/22 14:55:56 @@ -2094,6 +2094,7 @@ { int sock, newch; struct sockaddr_un sunaddr; + char *tmpdir; if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); @@ -2104,7 +2105,10 @@ /* Allocate a buffer for the socket name, and format the name. */ channel_forwarded_auth_socket_name = xmalloc(MAX_SOCKET_NAME); channel_forwarded_auth_socket_dir = xmalloc(MAX_SOCKET_NAME); - strlcpy(channel_forwarded_auth_socket_dir, "/tmp/ssh-XXXXXXXX", MAX_SOCKET_NAME); + if ((tmpdir = getenv("TMPDIR")) == NULL) + tmpdir = _PATH_TMP; + snprintf(channel_forwarded_auth_socket_dir, MAX_SOCKET_NAME, + "%s/ssh-XXXXXXXX", tmpdir); /* Create private directory for socket */ if (mkdtemp(channel_forwarded_auth_socket_dir) == NULL) { Index: session.c =================================================================== RCS file: /home/ncvs/src/crypto/openssh/session.c,v retrieving revision 1.4.2.5 diff -u -r1.4.2.5 session.c --- session.c 2000/10/28 23:00:49 1.4.2.5 +++ session.c 2000/12/22 14:55:56 @@ -184,7 +184,7 @@ int type, fd; int compression_level = 0, enable_compression_after_reply = 0; int have_pty = 0; - char *command; + char *command, *tmpdir; int n_bytes; int plen; unsigned int proto_len, data_len, dlen; @@ -324,7 +324,10 @@ /* Setup to always have a local .Xauthority. */ xauthfile = xmalloc(MAXPATHLEN); - strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN); + if ((tmpdir = getenv("TMPDIR")) == NULL) + tmpdir = _PATH_TMP; + snprintf(xauthfile, MAXPATHLEN, "%s/ssh-XXXXXXXX", + tmpdir); temporarily_use_uid(pw->pw_uid); if (mkdtemp(xauthfile) == NULL) { restore_uid(); @@ -1549,6 +1552,7 @@ session_x11_req(Session *s) { int fd; + char *tmpdir; if (no_x11_forwarding_flag) { debug("X11 forwarding disabled in user configuration file."); return 0; @@ -1579,7 +1583,9 @@ return 0; } xauthfile = xmalloc(MAXPATHLEN); - strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN); + if ((tmpdir = getenv("TMPDIR")) == NULL) + tmpdir = _PATH_TMP; + snprintf(xauthfile, MAXPATHLEN, "%s/ssh-XXXXXXXX", tmpdir); temporarily_use_uid(s->pw->pw_uid); if (mkdtemp(xauthfile) == NULL) { restore_uid(); Index: ssh-agent.1 =================================================================== RCS file: /home/ncvs/src/crypto/openssh/ssh-agent.1,v retrieving revision 1.1.1.1.2.2 diff -u -r1.1.1.1.2.2 ssh-agent.1 --- ssh-agent.1 2000/10/28 23:00:49 1.1.1.1.2.2 +++ ssh-agent.1 2000/12/22 14:55:56 @@ -129,6 +129,11 @@ .Ev SSH_AUTH_SOCK environment variable. +The +.Pa /tmp +directory can be overridden with the +.Ev TMPDIR +environment variable. The socket is made accessible only to the current user. This method is easily abused by root or another instance of the same user. Index: ssh-agent.c =================================================================== RCS file: /home/ncvs/src/crypto/openssh/ssh-agent.c,v retrieving revision 1.2.2.3 diff -u -r1.2.2.3 ssh-agent.c --- ssh-agent.c 2000/10/28 23:00:49 1.2.2.3 +++ ssh-agent.c 2000/12/22 14:55:56 @@ -87,8 +87,8 @@ pid_t parent_pid = -1; /* pathname and directory for AUTH_SOCKET */ -char socket_name[1024]; -char socket_dir[1024]; +char socket_name[MAXPATHLEN]; +char socket_dir[MAXPATHLEN]; extern char *__progname; @@ -655,7 +655,7 @@ int sock, c_flag = 0, k_flag = 0, s_flag = 0, ch; struct sockaddr_un sunaddr; pid_t pid; - char *shell, *format, *pidstr, pidstrbuf[1 + 3 * sizeof pid]; + char *shell, *format, *pidstr, pidstrbuf[1 + 3 * sizeof pid], *tmpdir; /* check if RSA support exists */ if (rsa_alive() == 0) { @@ -721,7 +721,9 @@ parent_pid = getpid(); /* Create private directory for agent socket */ - strlcpy(socket_dir, "/tmp/ssh-XXXXXXXX", sizeof socket_dir); + if ((tmpdir = getenv("TMPDIR")) == NULL) + tmpdir = _PATH_TMP; + snprintf(socket_dir, sizeof socket_dir, "%s/ssh-XXXXXXXX", tmpdir); if (mkdtemp(socket_dir) == NULL) { perror("mkdtemp: private socket dir"); exit(1); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message