From owner-freebsd-bugs@FreeBSD.ORG Fri Nov 30 07:09:39 2012 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 30171301 for ; Fri, 30 Nov 2012 07:09:39 +0000 (UTC) (envelope-from nvass@gmx.com) Received: from mailout-eu.gmx.com (mailout-eu.gmx.com [213.165.64.44]) by mx1.freebsd.org (Postfix) with SMTP id 8898F8FC0C for ; Fri, 30 Nov 2012 07:09:38 +0000 (UTC) Received: (qmail invoked by alias); 30 Nov 2012 07:09:31 -0000 Received: from 77.49.87.232.dsl.dyn.forthnet.gr (EHLO [192.168.1.77]) [77.49.87.232] by mail.gmx.com (mp-eu005) with SMTP; 30 Nov 2012 08:09:31 +0100 X-Authenticated: #46156728 X-Provags-ID: V01U2FsdGVkX18tU/s6xostX4ohAW95D+TMIikqbFT7PvbEplgRHC Aqn6lXUBkS+1c5 Message-ID: <50B85BA1.9030502@gmx.com> Date: Fri, 30 Nov 2012 09:09:21 +0200 From: Nikos Vassiliadis User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120604 Thunderbird/13.0 MIME-Version: 1.0 To: Bruce Evans Subject: Re: bin/173977: pw(8) does not do range-checking on UIDs/GUIs from user's input, passwd DB becomes inconsistent References: <201211281841.qASIfkx4033378@red.freebsd.org> <20121129090618.F1410@besplex.bde.org> In-Reply-To: <20121129090618.F1410@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 Cc: freebsd-bugs@freebsd.org X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Nov 2012 07:09:39 -0000 On 11/29/2012 12:32 AM, Bruce Evans wrote: > On Wed, 28 Nov 2012, Nikos Vassiliadis wrote: > >>> Description: >> pw(8) command does not do any range checking on the uid and gid input, resulting in inconsistencies in the password database. >> ... >>> Fix: >> >> >> Patch attached with submission follows: >> >> Index: usr.sbin/pw/pw_group.c >> =================================================================== >> --- usr.sbin/pw/pw_group.c (revision 243652) >> +++ usr.sbin/pw/pw_group.c (working copy) >> @@ -350,6 +350,8 @@ >> */ >> if (a_gid != NULL) { >> gid = (gid_t) atol(a_gid->val); > > The behaviour on overflow in atol() is undefined in C, but is benign > (the same as for strtol()) in FreeBSD. Then the behaviour on as > assignment to `gid' is undefined if sizeof(gid_t) < sizeof(long) > (64-bit systems in FreeBSD). > >> + if (errno == ERANGE || errno == EINVAL) >> + errx(EX_DATAERR, "gid %s is invalid", a_gid->val); > > Checking errno like this depends on atol() being essentially strtol(). > When checking like this, it is necessary to set errno to some value > (usually 0) different from all the error values that are checked for. > > The EINVAL check depends on an unportable POSIX feature. > > Garbage is still not detected on 64-bit systems. > >> >> if ((grp = GETGRGID(gid)) != NULL && getarg(args, 'o') == NULL) >> errx(EX_DATAERR, "gid `%ld' has already been allocated", (long) grp->gr_gid); > > Any program that uses atol() for parsing user input should be rmrf'ed of > course. There are a couple of programs than do gid conversions fairly > correctly. One is chpass(1). Its conversions are of low quality, but > not as low as in pw. It uses strtoul(), but then overflows by assigning > to uid_t or gid_t before checking that the value fits. It sets errno > before the call and checks ERANGE, but it also bogusly checks for ULONG_MAX > and thus bogusly rejects ids with that value. It does some up-front checking > to disallow ids that don't start with a digit, and disallows ids in octal > or hex. strtoul() would allow leading whitespace and a leading '-', and > cal be told to allow any base. For command line args that are numbers > represented as strings, I think it is a bug to not allow any format that > can be parsed, but for utilities operating on files like /etc/passwd, > only the standard format should be allowed. Thanks for the recommendations Bruce, I'll try to come up with something new soon. Nikos