From fa67c173e4fc7a2bc5a48cb973b99597afc26107 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 20 May 2024 21:53:39 -0400 Subject: [PATCH] Select between qubes.Filecopy and qubes.UnsafeFileCopy This adds a filesystem tree checker that determins if a directory tree can be copied with qubes.Filecopy or if qubes.UnsafeFileCopy is required. It also produces the total size of the tree, which qfile-agent uses for progress reporting. The checker can also be used for other purposes, such as checking if there are nasty characters or symbolic links in a filesystem tree unpacked via a command such as 'tar'. --- qubes-rpc/Makefile | 9 +- qubes-rpc/qubes-fs-tree-check.c | 363 ++++++++++++++++++++++++++++++++ qubes-rpc/qvm-copy | 44 +--- 3 files changed, 380 insertions(+), 36 deletions(-) create mode 100644 qubes-rpc/qubes-fs-tree-check.c diff --git a/qubes-rpc/Makefile b/qubes-rpc/Makefile index bf3020e3..fcc279cb 100644 --- a/qubes-rpc/Makefile +++ b/qubes-rpc/Makefile @@ -17,8 +17,10 @@ LDLIBS := -lqubes-rpc-filecopy .PHONY: all clean install -all: vm-file-editor qopen-in-vm qfile-agent qfile-unpacker tar2qfile +all: vm-file-editor qopen-in-vm qfile-agent qfile-unpacker tar2qfile qubes-fs-tree-check +qubes-fs-tree-check: LDLIBS := -lqubes-pure +qubes-fs-tree-check: qubes-fs-tree-check.o vm-file-editor: vm-file-editor.o qopen-in-vm: qopen-in-vm.o gui-fatal.o qfile-agent: qfile-agent.o gui-fatal.o @@ -26,7 +28,7 @@ qfile-unpacker: qfile-unpacker.o gui-fatal.o tar2qfile: tar2qfile.o gui-fatal.o clean: - -$(RM) -- qopen-in-vm qfile-agent qfile-unpacker tar2qfile vm-file-editor *.o + -$(RM) -- qopen-in-vm qfile-agent qfile-unpacker tar2qfile vm-file-editor qubes-fs-tree-check *.o install: install -d $(DESTDIR)$(BINDIR) @@ -46,7 +48,8 @@ install: install -t $(DESTDIR)$(QUBESLIBDIR) \ prepare-suspend resize-rootfs \ qfile-agent qopen-in-vm qrun-in-vm qubes-sync-clock \ - tar2qfile vm-file-editor xdg-icon qvm-template-repo-query + tar2qfile vm-file-editor xdg-icon qvm-template-repo-query \ + qubes-fs-tree-check # Install qfile-unpacker as SUID, because it will fail to receive # files from other vm. install -t $(DESTDIR)$(QUBESLIBDIR) -m 4755 qfile-unpacker diff --git a/qubes-rpc/qubes-fs-tree-check.c b/qubes-rpc/qubes-fs-tree-check.c new file mode 100644 index 00000000..803d5fe8 --- /dev/null +++ b/qubes-rpc/qubes-fs-tree-check.c @@ -0,0 +1,363 @@ +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +static bool +simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags, + unsigned long long *size); + +// This code operates on completely untrusted input. +// Therefore, it is critical to escape everything before displaying it. +// Use this function for that purpose. +char *simple_strvis(const char *untrusted_str) +{ + size_t len = strlen(untrusted_str); + if (len > PTRDIFF_MAX / 4 - 3) + errx(1, "String too long to escape"); + size_t out_len = len * 4 + 3; + char *p = malloc(out_len); + if (p == NULL) + err(1, "malloc(%zu)", out_len); + char *out_cursor = p; + char *out_end = p + out_len; + const char *in_cursor = untrusted_str; + *out_cursor++ = '"'; + for (;;) { + char c = *in_cursor++; + switch (c) { + case '\0': + assert(out_end - out_cursor >= 2); + *out_cursor++ = '"'; + *out_cursor++ = '\0'; + return p; + case '"': + case '\\': + assert(out_end - out_cursor >= 2); + *out_cursor++ = '\\'; + *out_cursor++ = in_cursor[-1]; + break; + case '\n': + assert(out_end - out_cursor >= 2); + *out_cursor++ = '\\'; + *out_cursor++ = 'n'; + break; + case '\t': + assert(out_end - out_cursor >= 2); + *out_cursor++ = '\\'; + *out_cursor++ = 't'; + break; + default: + assert(out_end - out_cursor >= 4); + if (c >= 0x20 && c <= 0x7E) + *out_cursor++ = c; + else { + *out_cursor++ = '\\'; + *out_cursor++ = '0' + ((uint8_t)c >> 6); + *out_cursor++ = '0' + ((uint8_t)c >> 3 & 7); + *out_cursor++ = '0' + ((uint8_t)c & 7); + } + } + } +} +bool machine = false; +bool ignore_symlinks = false; + +static bool +process_dirent(const char *d_name, int fd, int flags, const char *name, + bool ignore_symlinks, const char *escaped, unsigned long long *size) +{ + bool bad = false; + struct stat statbuf; + int res = fstatat(fd, d_name, &statbuf, AT_SYMLINK_NOFOLLOW); + if (res < 0) + err(1, "fstatat(%s)", escaped); + switch (statbuf.st_mode & S_IFMT) { + case S_IFLNK: + if (ignore_symlinks) + return false; // do not check path + if ((flags & COPY_ALLOW_SYMLINKS) == 0) + errx(1, "Cannot copy symbolic link %s", escaped); + break; + case S_IFDIR: + if ((flags & COPY_ALLOW_DIRECTORIES) == 0) + errx(1, "Cannot copy directory %s", escaped); + break; + case S_IFREG: + break; + default: + // In machine readable mode, ignore these + bad = !machine && (flags & COPY_ALLOW_UNSAFE_CHARACTERS) == 0 && + !qubes_pure_string_safe_for_display(d_name, 0); + if (bad) + warnx("%s is not safe for display", escaped); + return bad; + } + if ((flags & COPY_ALLOW_UNSAFE_CHARACTERS) == 0 && + !qubes_pure_string_safe_for_display(d_name, 0)) { + if (!machine) + warnx("%s is not safe for display", escaped); + bad = true; + } + if (S_ISDIR(statbuf.st_mode)) { + int sub_file = openat(fd, d_name, O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC | O_RDONLY); + if (sub_file < 0) + err(1, "open(%s)", escaped); + return simple_fs_walk(sub_file, ignore_symlinks, name, flags, size); + } else { + // __builtin_add_overflow uses infinite signed precision, + // so a negative number would not cause overflow. + if (statbuf.st_size < 0) + errx(1, "Negative size?"); + // If this overflows, then we definitely want to error out instead of + // filling up the receiver's disk. + if (__builtin_add_overflow(*size, statbuf.st_size, size)) + errx(1, "Refusing to copy over 2**64 bytes"); + if (S_ISREG(statbuf.st_mode) || ignore_symlinks) + return bad; + } + if (statbuf.st_size > SSIZE_MAX - 1) + errx(1, "symbolic link too large for readlink()"); + size_t link_size = (size_t)(statbuf.st_size + 1); + char *buf = malloc(link_size); + if (buf == NULL) + err(1, "malloc(%zu)", link_size); + ssize_t link_res = readlinkat(fd, d_name, buf, link_size); + if (link_res < 0) + err(1, "readlink(%s)", escaped); + if (link_res != statbuf.st_size) + errx(1, "readlink(%s) returned wrong size", escaped); + buf[link_res] = '\0'; // add NUL terminator + bad = !qubes_pure_string_safe_for_display(buf, 0); + // charset checks done already, do not repeat them + if (qubes_pure_validate_symbolic_link_v2((const uint8_t *)name, + (const uint8_t *)buf, + QUBES_PURE_ALLOW_UNSAFE_CHARACTERS) < 0) + errx(1, "Refusing to copy unsafe symbolic link %s", escaped); + free(buf); + return bad; +} + +static bool +simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags, + unsigned long long *size) +{ + DIR *dir = fdopendir(fd); + bool bad = false; + if (dir == NULL) + err(1, "fdopendir()"); + for (;;) { + errno = 0; + struct dirent *d = readdir(dir); + if (d == NULL) { + if (errno) + err(1, "readdir()"); + break; + } + if (strcmp(d->d_name, ".") == 0 || strcmp(d->d_name, "..") == 0) + continue; /* skip "." and ".." */ + char *full_name = d->d_name; + if (name != NULL && asprintf(&full_name, "%s/%s", name, d->d_name) < 1) + err(1, "asprintf() failed (out of memory?)"); + char *escaped = simple_strvis(full_name); + if (process_dirent(d->d_name, fd, flags, full_name, ignore_symlinks, + escaped, size)) + bad = true; + if (full_name != d->d_name) + free(full_name); + free(escaped); + } + closedir(dir); + return bad; +} + +// Normalize a path, removing duplicate slashes and "." components. +// The path is modified in-place. +bool normalize_path(char *arg) +{ + // cases that do _not_ end in / or /. are checked for below + bool must_be_directory = true; + size_t len = strlen(arg) + 1; + if (len < 2) + errx(1, "Argument is empty"); + if (arg[0] == '/') + errx(1, "Argument is absolute"); + // The input string is always at least as long as the output string, + // so operate in-place. + // Proof that this is sufficient: the only characters written to the + // output are either those from the input string (of which there are + // strlen(arg) before the NUL terminator) or slashes (of which at + // most one is inserted for every slash in the input). + const char *in_cursor = arg; + char *out_cursor = arg; + const char *end = arg + (len - 1); + // Loop through all path components. + // The string cannot start with '/' or '\0' due to earlier checks. + do { + // find the end of this path component + const char *next = strchr(in_cursor, '/'); + if (next == NULL) + next = end; + if (in_cursor >= next) { + // cannot happen, NUL and / are checked for before loop entry + // and in the loop exit condition + abort(); + } + size_t component_size = (size_t)(next - in_cursor); + // ".." is not allowed + if (component_size == 2 && memcmp(in_cursor, "..", 2) == 0) + errx(1, ".. component found in path"); + // is this component "."? + bool const is_dot = (component_size == 1 && in_cursor[0] == '.'); + // if this component is *not* ".", copy it + if (!is_dot) { + // Only add a slash if this is _not_ the first path component. + // Otherwise an input of e.g. "abc" causes buffer overflow! + if (out_cursor != arg) { + assert(in_cursor > out_cursor && "about to clobber data in use"); + assert(out_cursor < end && "about to clobber end"); + *out_cursor++ = '/'; + } + assert(in_cursor >= out_cursor); + // Copy the path component. Use memmove() as the source + // and destination might overlap, or even be the same + // (for paths like a/b/c/d that are already normalized). + memmove(out_cursor, in_cursor, component_size); + // advance the cursor + out_cursor += component_size; + } + + if (*next == '\0') { + // true for "a/.", false for "a" + must_be_directory = is_dot; + break; + } + // strchr() gurantees that *next is a slash, so skip it + in_cursor = next + 1; + // skip further slashes + while (in_cursor[0] == '/') + in_cursor++; + } while (in_cursor[0] != '\0'); + if (out_cursor == arg) + errx(1, "Path only contains \".\" components and slashes"); + *out_cursor = '\0'; + return must_be_directory; +} + +const struct option opts[] = { + {"machine-readable", no_argument, NULL, 'm'}, + {"ignore-symlinks", no_argument, NULL, 'n'}, + {"no-ignore-symlinks", no_argument, NULL, 's'}, + {"allow-symlinks", no_argument, NULL, 'a'}, + {"no-allow-symlinks", no_argument, NULL, 'A'}, + {"allow-directories", no_argument, NULL, 'd'}, + {"no-allow-directories", no_argument, NULL, 'D'}, + {"allow-unsafe-characters", no_argument, NULL, 'u'}, + {"no-allow-unsafe-characters", no_argument, NULL, 'U'}, + {0, 0, NULL, 0}, +}; + +int main(int argc, char **argv) +{ + if (argc < 2) + errx(1, "not enough arguments"); + const char *last; + int flags = 0; + for (;;) { + int longindex = sizeof(opts)/sizeof(opts[0]) - 1; + assert(optind >= 1 && optind <= argc); + last = argv[optind]; + int r = getopt_long(argc, argv, "+", opts, &longindex); + if (r == -1) + break; + switch (r) { + case ':': + case '?': + return 1; + case 'm': + machine = true; + break; + case 'n': + ignore_symlinks = true; + break; + case 's': + ignore_symlinks = false; + break; + case 'a': + flags |= COPY_ALLOW_SYMLINKS; + break; + case 'A': + flags &= ~COPY_ALLOW_SYMLINKS; + break; + case 'd': + flags |= COPY_ALLOW_DIRECTORIES; + break; + case 'D': + flags &= ~COPY_ALLOW_DIRECTORIES; + break; + case 'u': + flags |= COPY_ALLOW_UNSAFE_CHARACTERS; + break; + case 'U': + flags &= ~COPY_ALLOW_UNSAFE_CHARACTERS; + break; + default: + abort(); + } + } + if (argc <= optind) + errx(1, "No paths provided"); + assert(last != NULL); + if (strcmp(last, "--") != 0) { + for (int j = optind; j < argc; ++j) + if (argv[j][0] == '-') + errx(1, "argument %d begins with - but first argument is not --", j); + } + bool bad = false; + unsigned long long size = 0; + for (int i = optind; i < argc; ++i) { + size_t len = strlen(argv[i]) + 1; + if (len < 4 && memcmp("..", argv[i], len - 1) == 0) { + if (len == 0) + errx(1, "Empty string (passed as argument %d) is not a valid path", i); + else + errx(1, "Argument %d is \"%s\", which is not allowed. Try operating from the parent directory.", i, argv[i]); + } + if (argv[i][0] == '/') + errx(1, "Argument %d is an absolute path", i); + char *escaped = simple_strvis(argv[i]); + char *dup1 = malloc(len), *dup2 = malloc(len); + if (dup1 == NULL || dup2 == NULL) + err(1, "malloc(%zu)", len); + char *bname = basename(memcpy(dup1, argv[i], len)); + char *dname = dirname(memcpy(dup2, argv[i], len)); + size_t bname_len = strlen(bname); + if (bname_len < 3 && memcmp("..", bname, bname_len) == 0) + errx(1, "Refusing to copy path with basename empty, ., or .."); + int dir_fd = open(dname, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOCTTY); + if (dir_fd < 0) + err(1, "open(%s)", escaped); + if (process_dirent(bname, dir_fd, flags, argv[i], ignore_symlinks, + escaped, &size)) + bad = true; + free(dup2); + free(dup1); + free(escaped); + } + if (machine) + printf("%llu\n", size); + if (fflush(NULL) || ferror(stdout) || ferror(stderr)) + errx(1, "I/O error on standard stream"); + return bad ? 2 : 0; +} diff --git a/qubes-rpc/qvm-copy b/qubes-rpc/qvm-copy index 024d4bd0..298ac946 100755 --- a/qubes-rpc/qvm-copy +++ b/qubes-rpc/qvm-copy @@ -21,7 +21,7 @@ set -e -o pipefail -unset PROGRESS_TYPE OPERATION_TYPE TARGET_TYPE MIN_ARGS FILECOPY_TOTAL_SIZE +unset PROGRESS_TYPE OPERATION_TYPE TARGET_TYPE MIN_ARGS FILECOPY_TOTAL_SIZE service case ${0##*/} in (qvm-move) OPERATION_TYPE=move TARGET_TYPE=default MIN_ARGS=1;; @@ -74,40 +74,18 @@ else VM="@default" fi -for path; do - case $path in - (.|..) printf 'Refusing to copy "." or "..". -Try copying from the parent directory.\n' >&2; exit 1;; - esac -done - -if [ "$PROGRESS_TYPE" = console ] ; then - find_paths=( ) - for path; do - case "$path" in - (-*) find_paths+=( ./"$path" ) ;; - (*) find_paths+=( "$path" ) ;; - esac - done - - FILECOPY_TOTAL_SIZE=$( - find "${find_paths[@]}" -type f -print0 2>/dev/null | - du --files0-from - -c --apparent-size -k | - tail -n 1 | - cut -f 1 | - grep -xE '[0-9]+' - ) || FILECOPY_TOTAL_SIZE=0 - export FILECOPY_TOTAL_SIZE +if FILECOPY_TOTAL_SIZE=$(/usr/lib/qubes/qubes-fs-tree-check \ + --allow-symlinks --allow-directories --machine -- "$@"); then + service=qubes.Filecopy +else + status=$? + if [[ "$status" -ne 2 ]]; then exit "$status"; fi + service=qubes.UnsafeFileCopy fi +if [[ "$PROGRESS_TYPE" = 'console' ]]; then export FILECOPY_TOTAL_SIZE; fi -for path; do - if [ ! -e "$path" ]; then - echo "File or directory not found: $path" - exit 1 - fi -done - -/usr/lib/qubes/qrexec-client-vm --filter-escape-chars-stderr -- "$VM" qubes.Filecopy /usr/lib/qubes/qfile-agent "$@" +/usr/lib/qubes/qrexec-client-vm --filter-escape-chars-stderr -- "$VM" \ + "$service" /usr/lib/qubes/qfile-agent "$@" if [ "$OPERATION_TYPE" = "move" ] ; then rm -rf -- "$@"