Re: [hackers] [PATCH 1/3][sbase] chown: Ignore failure to find user/group name

From: Michael Forney <mforney_AT_mforney.org>
Date: Tue, 25 Jun 2019 16:55:10 -0700

On 2019-06-25, Richard Ipsum <richardipsum_AT_fastmail.co.uk> wrote:
> On Tue, Jun 25, 2019 at 03:44:57PM -0700, Michael Forney wrote:
>> On 2019-06-25, Richard Ipsum <richardipsum_AT_fastmail.co.uk> wrote:
>> > This fixes an error on OpenBSD,
>> >
>> > % chown 1000:1000 yes.c
>> > ./chown: getgrnam 1000: No such file or directory
>>
>> POSIX says that if the entry cannot be found, errno shall not be
>> changed. Are you saying that OpenBSD doesn't behave this way?
>
> Yeah when I run this on OpenBSD with uid:gid values I get ENOENT.

Do you think that's something they'd be willing to fix?

I think the error is coming from
https://github.com/openbsd/src/blob/f84583fe5d7899ab1504dc1d58a04742db4a3058/lib/libc/gen/getgrent.c#L217.
If I run `touch /var/run/ypbind.lock` in my OpenBSD VM, it fixes the
issue.

I think this is unintended, since there appears to be some logic to
save and restore errno in grscan(). Looks like it was introduced in
https://github.com/openbsd/src/commit/b971f1acd7c34a49359ccefbe512e06f3826a939
(first released in OpenBSD 5.9).

>>
>> > ---
>> > chown.c | 18 ++----------------
>> > 1 file changed, 2 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/chown.c b/chown.c
>> > index d7dc8e0..748ce97 100644
>> > --- a/chown.c
>> > +++ b/chown.c
>> > _AT_@ -79,26 +79,12 @@ main(int argc, char *argv[])
>> > *group++ = '\0';
>> >
>> > if (owner && *owner) {
>> > - errno = 0;
>> > pw = getpwnam(owner);
>> > - if (pw) {
>> > - uid = pw->pw_uid;
>> > - } else {
>> > - if (errno)
>> > - eprintf("getpwnam %s:", owner);
>> > - uid = estrtonum(owner, 0, UINT_MAX);
>> > - }
>> > + uid = pw ? pw->pw_uid : estrtonum(owner, 0, UINT_MAX);
>>
>> It looks like there is no error with getpwnam, only with getgrname,
>> right?
>>
>
> Yes, but I would argue that realistically the only time this function errors
> is
> when the entry doesn't exist, we don't want to exit in that case,
> instead try parsing it as a numeric uid and if that fails then exit.

The POSIX getgrnam specification also lists a couple other possible
errors: I/O error and out of file descriptors.

Also, the chown specification specifically says

        If a numeric group operand exists in the group database as a group
name, the group ID number associated with that group name shall be
used as the group ID.

If we aren't able to read /etc/group for some reason and the the group
name for a number exists, we would end up using the wrong group ID.
Received on Wed Jun 26 2019 - 01:55:10 CEST

This archive was generated by hypermail 2.3.0 : Wed Jun 26 2019 - 02:12:24 CEST