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.
--
: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);
