sandbox-app-launcher icon indicating copy to clipboard operation
sandbox-app-launcher copied to clipboard

Issues with mknod* seccomp-whitelist

Open rusty-snake opened this issue 3 years ago • 0 comments
trafficstars

TL;DR:

  • The whitelist for mknod does not mask out file permission bits making it rather useless.
  • The whitelist for mknodat checks the wrong argument making it (in theory) possible to call it with S_IFCHR/S_IFBLK.

Deep dive

seccomp-whitelist contains the following for mknod/mknodat:

# We don't need to allow creation of char/block devices.
mknod 1 S_IFREG
mknod 1 S_IFIFO
mknod 1 S_IFSOCK
mknodat 1 S_IFREG
mknodat 1 S_IFIFO
mknodat 1 S_IFSOCK

After running bash autogen-seccomp seccomp-whitelist this becomes

  ALLOW_ARG1 (mknod, S_IFREG);
  ALLOW_ARG1 (mknod, S_IFIFO);
  ALLOW_ARG1 (mknod, S_IFSOCK);
  ALLOW_ARG1 (mknodat, S_IFREG);
  ALLOW_ARG1 (mknodat, S_IFIFO);
  ALLOW_ARG1 (mknodat, S_IFSOCK);

Which gets expanded to the following (for x86_64) by running bash autogen-seccomp seccomp-whitelist | gcc -Wall -x c -E -P -:

  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0100000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0010000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0140000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0100000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0010000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0140000}), 0) < 0) goto out; };

Which looks like this if we manually revert the libseccomp macros:

  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFIFO), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFSOCK), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFIFO), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFSOCK), 0) < 0) goto out; };

The first issue here is that the mode argument (arg 1) of mknod

specifies both the file mode to use and the type of node to be created.

So it is only allowed to create a file with 000 permissions.

In order to check the type but not the mode all permission bit should be masked out like

-  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
+  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_MASKED_EQ, S_IFMT, S_IFREG), 0) < 0) goto out; };

The second issue is that mknodat is defined as int mknodat(int dirfd, const char *pathname, mode_t mode, dev_t dev); meaning mode is arg 2 and arg 1 is the pathname. This makes using mknodat pratically impossible (and mknod too because glibc implements mknod with mknodat) because it is only allowed if pathname is at S_IFREG, S_IFIFO or S_IFSOCK. And it makes it in theory possible to call mknodat with S_IFCHR/S_IFBLK though this seems to be very hard in practice because S_IFREG, S_IFIFO and S_IFSOCK are very low numbers.


Test programs

# Open autogen-seccomp and change 'seccomp_filter_path='
# Generate the seccomp-filter
bash autogen-seccomp seccomp-whitelist | gcc -Wall -l seccomp -o autogen-seccomp-whitelist -x c - && ./autogen-seccomp-whitelist`
# Comopile the test programs
gcc -Wall -o mknod mknod.c
gcc -Wall -o mknodat mknodat.c
# Run the test programs w/o seccomp-filter
./mknod
> mknod("/tmp/reg1", S_IFREG) = 0
> mknod("/tmp/reg2", S_IFREG|0644) = 0
./mknodat
> mknodat(AT_FDCWD, "/tmp/reg1", S_IFREG) = 0
> mknodat(AT_FDCWD, "/tmp/reg2", S_IFREG|0644) = 0
> ptr: c000
> ptr2: ffffffffffffffff    <== -1
> 1 Operation not permitted
> zsh: segmentation fault (core dumped)  ./mknodat
# Run the test programs with seccomp-filter
bwrap --dev-bind   --seccomp 3 ./mknod 3<./seccomp-whitelist.bpf
> mknod("/tmp/reg1", S_IFREG) = 0
> killed
bwrap --dev-bind   --seccomp 3 ./mknodat 3<./seccomp-whitelist.bpf
> killed

mknod.c:

#include <stdio.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <unistd.h>

// glibc uses mknodat to implement mknod (in newer versions).
int mknod(const char *pathname, mode_t mode, dev_t dev) {
    return syscall(SYS_mknod, pathname, mode, dev);
}

int main(int argc, char **argv) {
    int rv;

    unlink("/tmp/reg1");
    rv = mknod("/tmp/reg1", S_IFREG, 0);
    printf("mknod(\"/tmp/reg1\", S_IFREG) = %i\n", rv);

    unlink("/tmp/reg2");
    rv = mknod("/tmp/reg2", S_IFREG|0644, 0);
    printf("mknod(\"/tmp/reg2\", S_IFREG|0644) = %i\n", rv);
}

mknodat.c:

#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/sysmacros.h>
#include <unistd.h>

int main(int argc, char **argv) {
    int rv;

    unlink("/tmp/reg1");
    rv = mknodat(AT_FDCWD, "/tmp/reg1", S_IFREG, 0);
    printf("mknodat(AT_FDCWD, \"/tmp/reg1\", S_IFREG) = %i\n", rv);

    unlink("/tmp/reg2");
    rv = mknodat(AT_FDCWD, "/tmp/reg2", S_IFREG|0644, 0);
    printf("mknodat(AT_FDCWD, \"/tmp/reg2\", S_IFREG|0644) = %i\n", rv);

    char *ptr = (char *)S_IFSOCK;
    char *ptr2 = mmap(ptr, strlen("/tmp/reg3") + 1, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED, -1, 0);
    printf("ptr: %lx\nptr2: %lx\n%i %s\n", (uintptr_t)ptr, (uintptr_t)ptr2, errno, strerror(errno));
    strcpy(ptr, "/tmp/reg3"); // SEGV
    unlink("/tmp/reg3");
    rv = mknodat(AT_FDCWD, ptr, S_IFCHR, makedev(0, 0));
    printf("mknodat(AT_FDCWD, \"/tmp/reg3\", S_IFCHR, makedev(0, 0)) = %i\n", rv);
}

rusty-snake avatar May 23 '22 08:05 rusty-snake