Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Jul 2015 12:09:07 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Baptiste Daroussin <bapt@freebsd.org>
Cc:        Bruce Evans <brde@optusnet.com.au>, svn-src-head@freebsd.org,  svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r285985 - in head/usr.sbin/pw: . tests
Message-ID:  <20150729110330.O5544@besplex.bde.org>
In-Reply-To: <20150728232106.GG28638@ivaldir.etoilebsd.net>
References:  <201507282110.t6SLAx0k035167@repo.freebsd.org> <20150729080932.S5059@besplex.bde.org> <20150728232106.GG28638@ivaldir.etoilebsd.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 29 Jul 2015, Baptiste Daroussin wrote:

> On Wed, Jul 29, 2015 at 08:52:52AM +1000, Bruce Evans wrote:
>> On Tue, 28 Jul 2015, Baptiste Daroussin wrote:
>>
>>> Log:
>>>  Check uid/gid used when creating a user/group are not larger than UID_MAX/GID_MAX
>>>
>>>  PR:		173977
>>>  Reported by:	nvass@gmx.com
>>
>> This is broken in a different way than before.
>>
>>> Modified: head/usr.sbin/pw/pw.c
>>> ==============================================================================
>>> --- head/usr.sbin/pw/pw.c	Tue Jul 28 20:52:10 2015	(r285984)
>>> +++ head/usr.sbin/pw/pw.c	Tue Jul 28 21:10:58 2015	(r285985)
>>> @@ -269,7 +269,7 @@ main(int argc, char *argv[])
>>> 			}
>>> 			if (strspn(optarg, "0123456789") != strlen(optarg))
>>> 				errx(EX_USAGE, "-g expects a number");
>>> -			id = strtonum(optarg, 0, LONG_MAX, &errstr);
>>> +			id = strtonum(optarg, 0, GID_MAX, &errstr);
>>
>> `id' still has type long.  The assignment overflows on 32-bit arches when
>> the value exceeds 0x7fffffff.  That is for half of all valid values.  pw
>> is broken in not supporting these values, but at least it detected them
>> as errors in the previous version.  Old versions implemented this bug
>> using atoi() with no error checking.
>
> So writting a function like strtonum like function with that prototype
> intmax_t strtonumber(const char *, intmax_t min, intmax_t max, const char **);
> and an unsigned equivalent
> uintmax_t strtonumber(const char *, uintmax_t min, uintmax_t max, const char **);

Not completely, since this would restore some of the complications of the
strto*() family (there is API explosion, and types to match).  These 2
functions don't even handle all the integer ranges (with assymmetric
signedness), or hex numbers, or floating point).

To match the types for gid_t's with these APIs, you could write:

 	gid_t id = strtounum(optarg, 0, GID_MAX, &errstr);

or

 	uintmax_t id = strtounum(optarg, 0, GID_MAX, &errstr);

but can't mix these id types with others that need different signedness..
or use plain int/long/u_long for anything.  This is not a problem for
uids and gids since they usually have the same underlying type.

POSIX (XSI) made a mess of this for waitid(idtype_t idtype, id_t id,
...).  id can be either a pid, a uid, a gid or a session id.  So id_t
is specified as an integer type that can "contain" all of these.
Representing all of these is generally impossible due to the sign
differences, and the conversions made to "contain" seem to be
unspecified.  FreeBSD actually uses int64_t for id_t and an enum
for idtype_t, so simple conversions preserve the value at the cost
of bloat.  This wouldn't work with 64-bit uid_t.

A similar mess can be made for strto*num() by specifying it to work
with a container type num_t and making this type opaque so that it
is hard to use :-), except there no strict container exists for int64_t
and uint64_t.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150729110330.O5544>