On Wed, Dec 01, 2021 at 04:43:13PM +0100, Claudio Jeker wrote:
> On Wed, Dec 01, 2021 at 02:14:40PM +0100, Sebastien Marie wrote:
> > Hi,
> > 
> > I have a program with unexpected unveil violation.
> > 
> > I put the whole / read-only, and next few programs executable (the
> > purpose is to restrict the executable files to only a small set).
> > 
> > The directory containing the executable is not visible anymore.
> > 
> > $ cat test.c
> > #include <sys/stat.h>
> > 
> > #include <err.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > 
> > int
> > main(int argc, char *argv[])
> > {
> >     struct stat sb;
> > 
> >     if (unveil("/", "r") == -1)
> >             err(EXIT_FAILURE, "unveil: /");
> >     if (unveil("/usr/bin/id", "rx") == -1)
> >             err(EXIT_FAILURE, "unveil: /usr/bin/id");
> > 
> >     if (unveil(NULL, NULL) == -1)
> >             err(EXIT_FAILURE, "unveil");
> > 
> >     if (stat("/usr/bin", &sb) == -1)
> >             err(EXIT_FAILURE, "stat: /usr/bin");
> > 
> >     return EXIT_SUCCESS;
> > }
> > $ cc -Wall test.c
> > $ ./a.out
> > a.out: stat: /usr/bin: No such file or directory
> > 
> > If I explicity add `unveil("/usr/bin", "r")`, it is working as expected.
> > 
> > The order of unveil("/") and unveil("/usr/bin/id") doesn't change the
> > problem. unveil(NULL, NULL) isn't required for reproducing.
> > 
> 
> The problem is that /usr/bin is a unveil node to cover the terminal
> /usr/bin/id file. Now this node has an no access permissions set and
> returns an access violation. Now the could should actually fallback to the
> case where covering unveils are looked at.
> 
> The following diff seems to fix your case. Regress still passes as well.

This has been sitting around for a long time. What should we do with this?

The fix regards directories that have no uv_flags set as no match since they
only hold individual file unveil information. The code then passes down to
the end where a backtrack up the tree happens looking for the first node
that has some flags set.

-- 
:wq Claudio
 
diff --git sys/kern/kern_unveil.c sys/kern/kern_unveil.c
index 801c210c113..76fc1b3d4db 100644
--- sys/kern/kern_unveil.c
+++ sys/kern/kern_unveil.c
@@ -708,7 +708,7 @@ unveil_check_final(struct proc *p, struct nameidata *ni)
        if (ni->ni_vp != NULL && ni->ni_vp->v_type == VDIR) {
                /* We are matching a directory terminal component */
                uv = unveil_lookup(ni->ni_vp, pr, NULL);
-               if (uv == NULL) {
+               if (uv == NULL || (uv->uv_flags & UNVEIL_USERSET) == 0) {
 #ifdef DEBUG_UNVEIL
                        printf("unveil: %s(%d) no match for vnode %p\n",
                            pr->ps_comm, pr->ps_pid, ni->ni_vp);

Reply via email to