From aa0b85e5a45f4c877da6fde0e6c489d2a77b5d82 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 12 May 2024 10:30:35 -0400 Subject: [PATCH] Allow disabling symbolic link checking This turns off all checks for symbolic links, returning to the R4.1 behavior. This is unsafe, but it might be necessary in some cases. However, it will not be included by either qubes.Filecopy or qubes.UnsafeFileCopy. --- qrexec-lib/libqubes-rpc-filecopy.h | 1 + qrexec-lib/pure.h | 2 ++ qrexec-lib/unicode.c | 5 ++++- qrexec-lib/unpack.c | 2 +- qrexec-lib/validator-test.c | 6 ++++++ 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/qrexec-lib/libqubes-rpc-filecopy.h b/qrexec-lib/libqubes-rpc-filecopy.h index d9d7a235..608a4237 100644 --- a/qrexec-lib/libqubes-rpc-filecopy.h +++ b/qrexec-lib/libqubes-rpc-filecopy.h @@ -69,6 +69,7 @@ enum copy_flags { COPY_ALLOW_DIRECTORIES = (1 << 1), COPY_ALLOW_UNSAFE_CHARACTERS = (1 << 2), COPY_ALLOW_NON_CANONICAL_SYMLINKS = (1 << 3), + COPY_ALLOW_UNSAFE_SYMLINKS = (1 << 4), }; /* feedback handling */ diff --git a/qrexec-lib/pure.h b/qrexec-lib/pure.h index 1a5bee47..811ee54d 100644 --- a/qrexec-lib/pure.h +++ b/qrexec-lib/pure.h @@ -260,6 +260,8 @@ enum QubesFilenameValidationFlags { /// to be canonial, as they are assumed to come from a filesystem /// traversal, which will always produce canonical paths. QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS = (1 << 1), + /// Do not check symbolic links at all. + QUBES_PURE_ALLOW_UNSAFE_SYMLINKS = (1 << 2), /// Allow all paths to be non-canonical, including symlinks. QUBES_PURE_ALLOW_NON_CANONICAL_PATHS = (1 << 3), }; diff --git a/qrexec-lib/unicode.c b/qrexec-lib/unicode.c index 24cbc762..1f2c748a 100644 --- a/qrexec-lib/unicode.c +++ b/qrexec-lib/unicode.c @@ -223,7 +223,8 @@ static bool flag_check(const uint32_t flags) { int const allowed = (QUBES_PURE_ALLOW_UNSAFE_CHARACTERS | QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS | - QUBES_PURE_ALLOW_NON_CANONICAL_PATHS); + QUBES_PURE_ALLOW_NON_CANONICAL_PATHS | + QUBES_PURE_ALLOW_UNSAFE_SYMLINKS); return (flags & ~(__typeof__(flags))allowed) == 0; } @@ -255,6 +256,8 @@ qubes_pure_validate_symbolic_link_v2(const uint8_t *untrusted_name, ssize_t depth = validate_path(untrusted_name, 0, flags); if (depth < 0) return -EILSEQ; // -ENOLINK is only for symlinks + if ((flags & QUBES_PURE_ALLOW_UNSAFE_SYMLINKS) != 0) + return depth > 0 ? 0 : -ENOLINK; if ((flags & QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS) != 0) flags |= QUBES_PURE_ALLOW_NON_CANONICAL_PATHS; // Symlink paths must have at least 2 components: "a/b" is okay diff --git a/qrexec-lib/unpack.c b/qrexec-lib/unpack.c index 91b78b09..6b68c733 100644 --- a/qrexec-lib/unpack.c +++ b/qrexec-lib/unpack.c @@ -346,7 +346,7 @@ static void process_one_file(struct file_header *untrusted_hdr, int flags) // Never set QUBES_PURE_ALLOW_NON_CANONICAL_PATHS -- paths from qfile-agent // will always be canonical. uint32_t validate_flags = ((uint32_t)flags >> 2) & - (QUBES_PURE_ALLOW_UNSAFE_CHARACTERS | + (QUBES_PURE_ALLOW_UNSAFE_CHARACTERS | QUBES_PURE_ALLOW_UNSAFE_SYMLINKS | QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS); if (!read_all_with_crc(0, untrusted_namebuf, namelen)) do_exit(LEGAL_EOF, NULL); // hopefully remote has produced error message diff --git a/qrexec-lib/validator-test.c b/qrexec-lib/validator-test.c index 71b59e73..64c99142 100644 --- a/qrexec-lib/validator-test.c +++ b/qrexec-lib/validator-test.c @@ -172,6 +172,12 @@ int main(int argc, char **argv) TEST("a/b/c", "a/", 0, true), // Invalid paths are rejected... TEST("..", "a/", 0, false), + // ...even with QUBES_PURE_ALLOW_UNSAFE_SYMLINKS. + TEST("..", "a/", QUBES_PURE_ALLOW_UNSAFE_SYMLINKS, false), + // but QUBES_PURE_ALLOW_UNSAFE_SYMLINKS allows bad symlinks + TEST("a", "/home/user/.bashrc", QUBES_PURE_ALLOW_UNSAFE_SYMLINKS, true), + // that are otherwise rejected + TEST("a", "/home/user/.bashrc", 0, false), // QUBES_PURE_ALLOW_NON_CANONICAL_PATHS allows non-canonical symlinks... TEST("a/b", "b//c", QUBES_PURE_ALLOW_NON_CANONICAL_PATHS, true), // ...and non-canonical paths.