From 4210c63a5b1088eb442ded841475475416c6ef1f Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 9 Nov 2024 19:10:05 -0500 Subject: [PATCH 1/2] qubes-fs-tree-check: add --help This improves UX. --- qubes-rpc/qubes-fs-tree-check.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/qubes-rpc/qubes-fs-tree-check.c b/qubes-rpc/qubes-fs-tree-check.c index 741f4ea8..c75489d5 100644 --- a/qubes-rpc/qubes-fs-tree-check.c +++ b/qubes-rpc/qubes-fs-tree-check.c @@ -193,6 +193,7 @@ const struct option opts[] = { {"no-allow-directories", no_argument, NULL, 'D'}, {"allow-all-names", no_argument, NULL, 'u'}, {"no-allow-all-names", no_argument, NULL, 'U'}, + {"help", no_argument, NULL, 'h'}, {0, 0, NULL, 0}, }; @@ -240,6 +241,19 @@ int main(int argc, char **argv) case 'U': flags &= ~COPY_ALLOW_UNSAFE_CHARACTERS; break; + case 'h': + fputs("Usage:\n" + " --help Print this message\n" + " --machine-readable Print the number of bytes to copy on stdout\n" + " --ignore-symlinks Ignore symbolic links; overrides previous --no-ignore-symlinks\n" + " --no-ignore-symlinks Do not ignore symbolic links; overrides previous --ignore-symlinks\n" + " --allow-directories Allow directories; overrides previous --no-allow-directories\n" + " --no-allow-directories Do not allow directories; overrides previous --allow-directories\n" + " --allow-all-names Allow all-names; overrides previous --no-allow-all-names\n" + " --no-allow-all-names Do not allow all-names; overrides previous --allow-all-names\n", + stderr); + fflush(stderr); + return ferror(stderr) ? 1 : 0; default: abort(); } From 9004e7577033540d29760eb42b98231f4814ec20 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 9 Nov 2024 19:42:03 -0500 Subject: [PATCH 2/2] qubes-fs-tree-check: Detect if a directory is unsafe for display When processing directories, process_dirent() detected filenames that are unsafe for display and set "bad" to true, but "bad" was not used to determine the return value of process_dirent(). Instead, the function tail-called simple_fs_walk(). Therefore, directories with unsafe filenames were not detected. This caused qubes.Filecopy to be used instead of qubes.Filecopy+allow-all-names. qubes.Filecopy (correctly) rejects the directory. Fix the bug by ensuring that if bad is set to true, process_dirent() returns true even if the function recurses into simple_fs_walk(). This will cause qubes.Filecopy+allow-all-names to be used, which will accept the directory and allow the copy to succeed. Fixes: QubesOS/qubes-issues#9567 --- qubes-rpc/qubes-fs-tree-check.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qubes-rpc/qubes-fs-tree-check.c b/qubes-rpc/qubes-fs-tree-check.c index c75489d5..6c8e790b 100644 --- a/qubes-rpc/qubes-fs-tree-check.c +++ b/qubes-rpc/qubes-fs-tree-check.c @@ -115,7 +115,9 @@ process_dirent(const char *d_name, int fd, int flags, const char *name, int sub_file = openat(fd, d_name, O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC | O_RDONLY); if (sub_file < 0) err(1, "open(%s)", escaped); - return simple_fs_walk(sub_file, ignore_symlinks, name, flags, size); + // If "bad" is true, return "true", but still do the + // FS walk to get the amount of data to copy. + return simple_fs_walk(sub_file, ignore_symlinks, name, flags, size) || bad; } else { // __builtin_add_overflow uses infinite signed precision, // so a negative number would not cause overflow.