From 9d625db6aff0c87be5547b5a567e99952ba91513 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 9 May 2024 12:09:15 -0400 Subject: [PATCH] qfile-unpacker: Add flags to allow unsafe characters and symlinks This requires QubesOS/qubes-linux-utils#113. It also adds a new argument parser based on getopt_long(), which is used instead of the old hand-rolled code unless there are at least two arguments and the first one starts with an ASCII digit. Part of QubesOS/qubes-issues#8332 --- qubes-rpc/qfile-unpacker.c | 160 ++++++++++++++++++++++++++++++------- 1 file changed, 131 insertions(+), 29 deletions(-) diff --git a/qubes-rpc/qfile-unpacker.c b/qubes-rpc/qfile-unpacker.c index 2ec3b333..672ff6e2 100644 --- a/qubes-rpc/qfile-unpacker.c +++ b/qubes-rpc/qfile-unpacker.c @@ -11,8 +11,13 @@ #include #include #include -#include #include +#include +#include +#include +#include + +#include #include #define INCOMING_DIR_NAME "QubesIncoming" @@ -41,6 +46,55 @@ static char *prepare_creds_return_dir(uid_t uid, uid_t myuid) return pwd->pw_dir; } +static void set_wait_for_space_str(const char *str) +{ + if (strcmp(str, "0") == 0) { + set_wait_for_space(0); + return; + } + if (str[0] >= '1' && str[0] <= '9') { + errno = 0; + char *endp; + long res = strtol(str, &endp, 10); + if (errno == 0 && *endp == '\0' && res > 0 && res <= INT_MAX) { + set_wait_for_space((int)res); + return; + } + } + errx(1, "Space amount %s is invalid or exceeds %d bytes", str, INT_MAX); +} + +enum { + opt_allow_unsafe_characters = 256, + opt_allow_unsafe_symlinks, + opt_no_allow_unsafe_characters, + opt_no_allow_unsafe_symlinks, +}; + +const struct option opts[] = { + { "no-allow-unsafe-characters", no_argument, NULL, opt_no_allow_unsafe_characters }, + { "allow-unsafe-characters", no_argument, NULL, opt_allow_unsafe_characters }, + { "no-allow-unsafe-symlinks", no_argument, NULL, opt_no_allow_unsafe_symlinks }, + { "allow-unsafe-symlinks", no_argument, NULL, opt_allow_unsafe_symlinks }, + { "verbose", no_argument, NULL, 'v' }, + { "wait-for-space", required_argument, NULL, 'w' }, + { NULL, 0, NULL, 0 }, +}; + +uid_t parse_uid(const char *user) +{ + if (strcmp(user, "0") == 0) + return 0; + errno = 0; + char *end = NULL; + unsigned long long u = strtoull(user, &end, 10); + uid_t uid = (uid_t)u; + if (user[0] < '1' || user[0] > '9' || + errno != 0 || *end != '\0' || uid != u) + gui_fatal("Invalid user ID argument"); + return uid; +} + int main(int argc, char ** argv) { char *home_dir; @@ -52,23 +106,84 @@ int main(int argc, char ** argv) char *procdir_path; int procfs_fd; int i, ret; + int flags = COPY_ALLOW_SYMLINKS | COPY_ALLOW_DIRECTORIES; + if (argc < 1) + errx(EXIT_FAILURE, "NULL argv[0] passed to execve()"); + if (argc >= 3 && argv[1][0] >= '0' && argv[1][0] <= '9') { + // Legacy case: parse options by hand + uid = parse_uid(argv[1]); + incoming_dir = argv[2]; - if (argc >= 3) { - char *end, *user = argv[1]; - errno = 0; - if (strcmp(user, "0") != 0) { - unsigned long long u = strtoull(user, &end, 10); - uid = (uid_t)u; - if (user[0] < '1' || user[0] > '9' || - errno != 0 || *end != '\0' || uid != u) - gui_fatal("Invalid user ID argument"); - } else { - uid = 0; + for (i = 3; i < argc; i++) { + if (strcmp(argv[i], "-v") == 0) + set_verbose(1); + else if (strcmp(argv[i], "-w") == 0) { + const char *next = argv[i + 1]; + if (next != NULL && next[0] != '-') { + set_wait_for_space_str(next); + i++; + } else { + set_wait_for_space(1); + } + } else { + gui_fatal("Invalid option %s", argv[i]); + } } - home_dir = prepare_creds_return_dir(uid, caller_uid); - incoming_dir = argv[2]; } else { - home_dir = prepare_creds_return_dir(caller_uid, caller_uid); + incoming_dir = NULL; + // Modern case: use getopt(3) + for (;;) { + if (optind < 1 || optind > argc) { + // FIXME: is this actually impossible? + assert(!"invalid optind() value?"); + abort(); + } + int longindex = -1; + const char *const last = argv[optind]; + int opt = getopt_long(argc, argv, "+vw:", opts, &longindex); + if (opt == -1) { + if (argc <= optind) + break; + if (argc - optind > 2) + errx(1, "Wrong number of non-option arguments (expected no more than 2, got %d)", + argc - optind); + if (argv[optind][0] != '\0') + uid = parse_uid(argv[optind]); + // might be NULL + incoming_dir = argv[optind + 1]; + break; + } + if (opt == '?' || opt == ':') + return EXIT_FAILURE; + if (longindex != -1) { + const char *expected = opts[longindex].name; + if (strncmp(expected, last + 2, strlen(expected)) != 0) + errx(1, "Option %s must be passed as --%s", last, expected); + } + switch (opt) { + case 'v': + set_verbose(1); + break; + case opt_allow_unsafe_characters: + flags |= COPY_ALLOW_UNSAFE_CHARACTERS; + break; + case opt_allow_unsafe_symlinks: + flags |= COPY_ALLOW_UNSAFE_SYMLINKS; + break; + case opt_no_allow_unsafe_characters: + flags &= ~COPY_ALLOW_UNSAFE_CHARACTERS; + break; + case opt_no_allow_unsafe_symlinks: + flags &= ~COPY_ALLOW_UNSAFE_SYMLINKS; + break; + case 'w': + set_wait_for_space_str(optarg); + break; + } + } + } + home_dir = prepare_creds_return_dir(uid, caller_uid); + if (incoming_dir == NULL) { remote_domain = getenv("QREXEC_REMOTE_DOMAIN"); if (!remote_domain) { gui_fatal("Cannot get remote domain name"); @@ -85,19 +200,6 @@ int main(int argc, char ** argv) mkdir(incoming_dir, 0700); } - for (i = 3; i < argc; i++) { - if (strcmp(argv[i], "-v") == 0) - set_verbose(1); - else if (strcmp(argv[i], "-w") == 0) - if (i+1 < argc && argv[i+1][0] != '-') { - set_wait_for_space(atoi(argv[i+1])); - i++; - } else - set_wait_for_space(1); - else - gui_fatal("Invalid option %s", argv[i]); - } - if (chdir(incoming_dir)) gui_fatal("Error chdir to %s", incoming_dir); @@ -129,7 +231,7 @@ int main(int argc, char ** argv) perror("setuid"); exit(1); } - return do_unpack(); + return do_unpack_ext(flags); } if (waitpid(pid, &ret, 0) < 0) { gui_fatal("Failed to wait for child process");