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.
  • Loading branch information
DemiMarie committed May 12, 2024
1 parent cc59ab4 commit da6601c
Show file tree
Hide file tree
Showing 5 changed files with 12 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 @@ -225,7 +225,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 @@ -257,6 +258,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 @@ -343,7 +343,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
4 changes: 4 additions & 0 deletions qrexec-lib/validator-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ 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", "/bin/sh", QUBES_PURE_ALLOW_UNSAFE_SYMLINKS, true),
// QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS allows non-canonical symlinks...
TEST("a/b", "b//c", QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS, true),
TEST("a/b", "b/./c", QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS, true),
Expand Down

0 comments on commit da6601c

Please sign in to comment.