Re: [hackers] [sbase] [PATCH] ls: allow listing contents of directories with +r-x

From: David Phillips <david_AT_sighup.nz>
Date: Sun, 22 Oct 2017 11:33:09 +1300

On Fri, Oct 20, 2017 at 01:28:30PM -0700, Michael Forney wrote:
> Hi David,

Hi Michael

>
> Can you explain the reasoning behind this? Is this behavior
> standardized, or consistent with other ls(1) implementations?
>

The purpose for this behaviour was to allow stat to fail when it is
used to find properties of files in a directory. For example:

    $ mkdir foo
    $ touch foo/bar
    $ chmod 600 tmp
    $ ls tmp
    ls: lstat foo/bar: Permission denied
    ?--------- 0 root root 0 Jan 01 1970 bar

Being able to at least see the names of the files that are contained
in a directory where we have no execute permission may be useful,
but (l)stat failure normally results in a fatal error for ls.

Regarding how standard this behaviour is, I've not read up on this in
the POSIX standard, but it is the way that another ls implementation
appears to behave (GNU). Thanks for bringing this point up though,
since I just tested on a SunOS machine and found it to simply report
'total 0', something I didn't realise it did.

Additionally, the output in such a case is probably another issue
that I have only just noticed (all zeroes → root:root 1970-01-01).

> > + if (prefix != NULL)
> > + snprintf(path, PATH_MAX, "%s/%s", prefix, name);
>
> You need to check whether this returned an error, or the result was truncated.
>

You're right. There are many other parts of the code that use snprintf
without error checking. These others might be the topic for another
patch, but I agree that this patch should at least be modified to
check for error.

> > + else
> > + strncpy(path, name, PATH_MAX);
>
> strncpy is almost never what you want. It does not null-terminate the
> string if it doesn't fit in the destination, and if it is shorter than
> PATH_MAX, the remaining buffer will be filled with zeros.
>

Definitely a good catch here too, I hadn't remembered that limitation
of strncpy. Once the other points are discussed, I will resubmit a
patch that uses strlcpy.


Grateful for your notes,
David
Received on Sun Oct 22 2017 - 00:33:09 CEST