svn commit: r243076 - head/usr.sbin/chkgrp

[Bruce Evans]

On Thu, 15 Nov 2012, Konstantin Belousov wrote:

> On Thu, Nov 15, 2012 at 01:52:46PM -0500, Eitan Adler wrote:
>> On 15 November 2012 11:52, Bruce Evans <brde at optusnet.com.au> wrote:
>>> strtoul("1garbage", NULL, 10) succeeds and returns value 1, but the input
>>> is garbage.
>>
>> This case is covered earlier
>> 160         /* check that the GID is numeric */
>> 161         if (strspn(f[2], "0123456789") != strlen(f[2])) {
>> 162             warnx("%s: line %d: GID is not numeric", gfn, n);
> So this code shall be removed, if you are introducing strtoul() to check
> for errors at all.

It is indeed strange to mix methods like this, and the check is more
strict that usual since it doesn't allow leading spaces.  I think leading
spaces are normally allowed for numeric args.  This normally happens
automatically, via bad programs using atoi() and better programs using
strto*().  Extra work like the above has to be done to reject leading
spaces.

>>> As the man page says, the EINVAL feature is unportable.  It is almost
>>> useless, since to detect garbage after the number you have to pass an
>>> endptr to strtoul(), and then the check for no conversion (that is,
>>> for garbage at the beginning) is just as easy as the check for garbage
>>> at the end.
>>
>> This patch doesn't care about EINVAL or ERANGE. It just cares strtoul
>> returned an error.

The only possible error for strtol() on a string of digits is ERANGE.

>> I even considered just ignoring the error case because the data is
>> mostly sanity checked prior.

The prior checking doesn't detect range errors.

The later checking detects range errors on 64-bit systems but not on 32-bit
ones, since strtoul() returns ULONG_MAX for range errors and that is a
valid gid (GID_MAX) on 32-bit systems.

Bruce