Skip to content

Commit

Permalink
Allow disabling symbolic link checking
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit aa0b85e)
  • Loading branch information
DemiMarie authored and marmarek committed Jun 22, 2024
1 parent 9912ad6 commit 641ba4d
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 2 deletions.
1 change: 1 addition & 0 deletions qrexec-lib/libqubes-rpc-filecopy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions qrexec-lib/pure.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
Expand Down
5 changes: 4 additions & 1 deletion qrexec-lib/unicode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion qrexec-lib/unpack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions qrexec-lib/validator-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 641ba4d

Please sign in to comment.