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.