From owner-freebsd-security  Mon May 12 07:32:09 1997
Return-Path: <owner-security>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.5/8.8.5) id HAA13969
          for security-outgoing; Mon, 12 May 1997 07:32:09 -0700 (PDT)
Received: from khavrinen.lcs.mit.edu (khavrinen.lcs.mit.edu [18.24.4.193])
          by hub.freebsd.org (8.8.5/8.8.5) with ESMTP id HAA13964
          for <freebsd-security@FreeBSD.ORG>; Mon, 12 May 1997 07:32:04 -0700 (PDT)
Received: (from wollman@localhost)
	by khavrinen.lcs.mit.edu (8.8.5/8.8.5) id KAA18551;
	Mon, 12 May 1997 10:31:48 -0400 (EDT)
Date: Mon, 12 May 1997 10:31:48 -0400 (EDT)
From: Garrett Wollman <wollman@khavrinen.lcs.mit.edu>
Message-Id: <199705121431.KAA18551@khavrinen.lcs.mit.edu>
To: Wolfram Schneider <wosch@apfel.de>
Cc: Gnuchev Fedor <qwe@ht.eimb.rssi.ru>, freebsd-security@FreeBSD.ORG
Subject: Re: Linux UID/GID 'Feature'
In-Reply-To: <p1ibu6i2d6x.fsf@campa.panke.de>
References: <Pine.BSF.3.95q.970511134602.168C-100000@ht.eimb.rssi.ru>
	<p1iwwp65918.fsf@campa.panke.de>
	<p1ibu6i2d6x.fsf@campa.panke.de>
Sender: owner-security@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

<<On 11 May 1997 18:20:06 +0200, Wolfram Schneider <wosch@apfel.de> said:

>  	id = atol(p);
> +	for(; *p != '\0'; p++)
> +		if (!isdigit(*p))
> +			goto fmt;
> +

This is why you should never use atol().  Always, always, always use
strtol(), and then you won't have these problems.  Properly written to
use strtol:

	errno = 0;
	ltmp = strtol(p, &ep, 10);
	if (*ep != '\0' || ltmp > MAX_UID_VALUE || ltmp < MIN_UID_VALUE 
	    || errno != 0) {
		do_error_action();
	}
	id = ltmp;

The errno check is not necessary if you can always guarantee that
MAX_UID_VALUE is strictly less than LONG_MAX and similarly
MIN_UID_VALUE is strictly greater than LONG_MIN.  Careful programmers
would leave it in anyway, since people who make such guarantees cannot
be trusted :-) .

>  	if (id > USHRT_MAX) {
>  		warnx("%s > max gid value (%d)", p, USHRT_MAX);
>  		/* return (0); This should not be fatal! */

This is really evil.  The pw_mkdb program should not have built into
it the identity of the type which is u/gid_t.  Rather, the constants I
mentioned above should be carefully defined somewhere (probably in
<pwd.h> under the non-POSIX section).

-GAWollman

--
Garrett A. Wollman   | O Siem / We are all family / O Siem / We're all the same
wollman@lcs.mit.edu  | O Siem / The fires of freedom 
Opinions not those of| Dance in the burning flame
MIT, LCS, CRS, or NSA|                     - Susan Aglukark and Chad Irschick