Reopening a closed PR... bin/63160

[Bruce Evans]

On Thu, 3 Nov 2005, Thordur I. Bjornsson wrote:

> I'm not sure how you reopen a PR or if you are supposed to send a new PR

I think only committers can reopen a PR, so it is easier for non-committers
to send a new one.

> Here goes,
> I'm not able to chown a dir/file what-have-you with the user nobody
> e.g
> [thib at caulfield ~]$ chown nobody foo
> chown: nobody: Invalid argument
>
> [thib at caulfield ~]$ fgrep nobody /etc/passwd
> nobody:*:65534:65534:Unprivileged user:/nonexistent:/usr/sbin/nologin
>
> I rebuild chown with CFLAGS= -g and verified that this is the same
> proplem as bin/63160
> <gdb session snipplet>
> (gdb) step
> 261             val = strtoul(name, &ep, 10);
> (gdb) step
> 262             if (errno)
> (gdb) step
> 263                     err(1, "%s", name);
> (gdb) step
> chown: nobody: Invalid argument

This has very little to do with chown(1) or strtoul(3).  Their error handling
is just slightly broken.  chown(1) only gets here when it thinks `name' is
a numeric id.  The main problem occurs earlier when getpwnam() fails to
find "norbody".  From an old (?) version of chown.c:

% void
% a_uid(const char *s)
% {
% 	struct passwd *pw;
% 
% 	if (*s == '\0')			/* Argument was "[:.]gid". */
% 		return;
% 	uid = ((pw = getpwnam(s)) != NULL) ? pw->pw_uid : id(s, "user");
% }

This and a_gid() are the only places that call id().  Normally getpwnam()
succeeds and id() is not used.

% uid_t
% id(const char *name, const char *type)
% {
% 	uid_t val;
% 	char *ep;
% 
% 	/*
% 	 * XXX
% 	 * We know that uid_t's and gid_t's are unsigned longs.
% 	 */
% 	errno = 0;
% 	val = strtoul(name, &ep, 10);

This strtoul() on "nobody" is certain to fail, but the error handling is
not quite right and has been broken.

% 	if (errno)
% 		err(1, "%s", name);
% 	if (*ep != '\0')
% 		errx(1, "%s: illegal %s name", name, type);
% 	return (val);
% }

strtoul() was changed to set errno to EINVAL if no conversion could be
performed.  This is a POSIX extension that is incompatible with C90 and
C99.  It breaks old code like the above.  The above now fails to print
the intended error message ("chown": nobody: illegal user name" here)
in all cases where `name' doesn't start with a number.  The only possible
nonzero errno from strtoul used to be ERANGE, so the above only failed to
print the intended error message when `name' does start with a number but
that number is too large to fit in an unsigned long.  The above should
print the type ("user" or "group") in both error messages and should say
something more useful than "illegal" (e.g., "nobody: no such user",
"1234: no such user", "1234foo: invalid numeric user id", "-2: invalid
numeric user id" (note that strtoul() won't set ERANGE for this), and
"5000000000: user id too large").

Other bugs in id():
- wrong comment.  We don't know that uid_t's and gid_t's are unsigned longs.
   They used to be in 4.4BSD-Lite1 and FreeBSD-2.0, but this assumed that all
   arches are vax.  This was fixed in 4.4BSD-Lite2 (earlier in NetBSD).
   They are now uint32_t in FreeBSD, and several arches have 64-bit u_longs.
- wrong error handling associated with the wrong comment.  strtoul() sets
   errno to ERANGE if `name' is a too-large number.  We carefully check for
   this, and then blindly truncate the value (if it is larger than u_long)
   by implicitly assigning it to a uid_t on return.

I checked my changes to mknod(2) for how to use strtoul(), and found
id() and its bugs duplicated there.  These days strotoumax() should
almost always be used for parsing numbers.

Bruce