From owner-freebsd-security  Mon May 12 09:17:21 1997
Return-Path: <owner-security>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.5/8.8.5) id JAA19170
          for security-outgoing; Mon, 12 May 1997 09:17:21 -0700 (PDT)
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19])
          by hub.freebsd.org (8.8.5/8.8.5) with ESMTP id JAA19165
          for <freebsd-security@FreeBSD.ORG>; Mon, 12 May 1997 09:17:18 -0700 (PDT)
Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.5/8.6.9) id CAA16319; Tue, 13 May 1997 02:15:23 +1000
Date: Tue, 13 May 1997 02:15:23 +1000
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199705121615.CAA16319@godzilla.zeta.org.au>
To: wollman@khavrinen.lcs.mit.edu, wosch@apfel.de
Subject: Re: Linux UID/GID 'Feature'
Cc: freebsd-security@FreeBSD.ORG, qwe@ht.eimb.rssi.ru
Sender: owner-security@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

>>  	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;

MAX_UID_VALUE is 0xffffffff, so it can only be read using strtol() on
systems with more than 32 bits in an int.  This is why you should rarely
use strtol() :-).  Always use strtoul() or strtouq() to read unsigned
values.  These functions are often more convenient even for reading
possibly-signed values.

Another problem: isdigit(*p) is usually undefined if *p < 0.

>>  	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).

Well, it needs to know something about the type, or depend on the
constants being representable by the type returned by the strto*
function used.  This is difficult to program POSIX-portably, since
uid_t might be long double.

Bruce