Date: Fri, 16 Nov 2012 22:38:30 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Eitan Adler <eadler@freebsd.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r243076 - head/usr.sbin/chkgrp Message-ID: <20121116220347.S1856@besplex.bde.org> In-Reply-To: <20121115185958.GG73505@kib.kiev.ua> References: <201211151506.qAFF63v0003848@svn.freebsd.org> <20121115153030.GD73505@kib.kiev.ua> <CAF6rxgk9w0_Qwo=92g-OSe5imvupG8qg7DzpCS9UVxzwMjn20g@mail.gmail.com> <20121116032851.I44199@besplex.bde.org> <CAF6rxgkvh_QEw5o3-8rznkEQMvQHY=ngYVM-7HQ11jHwX2vZ=w@mail.gmail.com> <20121115185958.GG73505@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121116220347.S1856>