From ace0cde3d3734b2327ab97fa71984aedfe2c21c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 10:50:05 +0000 Subject: [PATCH 1/2] fm-shim: D-Bus forwarding proxy, fix name ownership, add env sync Root cause fixes for fm-shim issues on Qubes OS: 1. Backend: Transform from full intercepting proxy into D-Bus activation proxy (marmarek's suggestion). On confirmed request, release the FileManager1 name, start pcmanfm-qt --daemon-mode, and forward the request via D-Bus ShowFolders. This eliminates the TOCTOU vulnerability that existed with CLI invocation (gio launch). 2. Backend: Fix name ownership handling. When another process already owns org.freedesktop.FileManager1 (e.g. pcmanfm-qt in daemon mode), send READY=1 so systemd does not time out the service. The shim stays queued and takes over when the current owner exits. 3. Frontend: Simplify to confirmation-only dialog. Exit code 0 = user confirmed (backend handles D-Bus forwarding), exit code 1 = user cancelled. Remove all xdg-mime query and gio launch code. 4. Add XDG autostart entry that runs dbus-update-activation-environment --systemd --all at session start. This syncs the session environment to systemd's user manager, fixing the stale XDG_DATA_DIRS that caused xdg-mime to return Catfish instead of pcmanfm-qt on Qubes OS. 5. Add D-Bus service activation file for org.freedesktop.FileManager1 so the shim can be started on-demand by D-Bus. https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233 https://claude.ai/code/session_01SAkV8U9vft3vzZAyJfzXGE --- debian/security-misc-shared.install | 2 + ...dbus-env-sync.desktop#security-misc-shared | 23 + .../fm_shim_frontend.py#security-misc-shared | 139 +----- ....FileManager1.service#security-misc-shared | 14 + .../fm-shim-backend.c#security-misc-shared | 401 ++++++++++++++---- 5 files changed, 362 insertions(+), 217 deletions(-) create mode 100644 etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared create mode 100644 usr/share/dbus-1/services/org.freedesktop.FileManager1.service#security-misc-shared diff --git a/debian/security-misc-shared.install b/debian/security-misc-shared.install index 3703ee9d..f63ef079 100755 --- a/debian/security-misc-shared.install +++ b/debian/security-misc-shared.install @@ -6,6 +6,7 @@ ## This file was generated using 'genmkfile debinstfile'. etc/apparmor.d/tunables/home.d/security-misc#security-misc-shared => /etc/apparmor.d/tunables/home.d/security-misc +etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared => /etc/xdg/autostart/security-misc-dbus-env-sync.desktop etc/apt/apt.conf.d/40error-on-any#security-misc-shared => /etc/apt/apt.conf.d/40error-on-any etc/apt/apt.conf.d/40sandbox#security-misc-shared => /etc/apt/apt.conf.d/40sandbox etc/default/grub.d/40_cpu_mitigations.cfg#security-misc-shared => /etc/default/grub.d/40_cpu_mitigations.cfg @@ -126,6 +127,7 @@ usr/libexec/security-misc/panic-on-oops#security-misc-shared => /usr/libexec/sec usr/libexec/security-misc/permission-lockdown#security-misc-shared => /usr/libexec/security-misc/permission-lockdown usr/libexec/security-misc/remove-system.map#security-misc-shared => /usr/libexec/security-misc/remove-system.map usr/libexec/security-misc/virusforget#security-misc-shared => /usr/libexec/security-misc/virusforget +usr/share/dbus-1/services/org.freedesktop.FileManager1.service#security-misc-shared => /usr/share/dbus-1/services/org.freedesktop.FileManager1.service usr/share/doc/security-misc/fstab-vm#security-misc-shared => /usr/share/doc/security-misc/fstab-vm usr/share/glib-2.0/schemas/30_security-misc.gschema.override#security-misc-shared => /usr/share/glib-2.0/schemas/30_security-misc.gschema.override usr/share/lintian/overrides/security-misc-shared#security-misc-shared => /usr/share/lintian/overrides/security-misc-shared diff --git a/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared b/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared new file mode 100644 index 00000000..f2532ac6 --- /dev/null +++ b/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared @@ -0,0 +1,23 @@ +## Copyright (C) 2026 - 2026 ENCRYPTED SUPPORT LLC +## See the file COPYING for copying conditions. + +## Synchronize session environment variables to the D-Bus activation +## environment and systemd's user manager. This ensures that services +## started by systemd or D-Bus activation (such as fm-shim) have +## access to the correct DISPLAY, XDG_DATA_DIRS, and other session +## variables. +## +## This is necessary on Qubes OS where the session setup does not +## automatically sync the environment, causing services to operate +## with stale or missing variables. Standard desktop environments +## (GNOME, KDE) perform this sync as part of their session startup. +## +## https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233 + +[Desktop Entry] +Type=Application +Name=D-Bus Environment Sync +Comment=Sync session environment to D-Bus and systemd user manager +Exec=/bin/sh -c 'dbus-update-activation-environment --systemd --all' +StartupNotify=false +NoDisplay=true diff --git a/usr/lib/python3/dist-packages/fm_shim_frontend/fm_shim_frontend.py#security-misc-shared b/usr/lib/python3/dist-packages/fm_shim_frontend/fm_shim_frontend.py#security-misc-shared index aef54ef1..da5c8096 100644 --- a/usr/lib/python3/dist-packages/fm_shim_frontend/fm_shim_frontend.py#security-misc-shared +++ b/usr/lib/python3/dist-packages/fm_shim_frontend/fm_shim_frontend.py#security-misc-shared @@ -9,29 +9,28 @@ ## ## * Local denial-of-service is out of scope. There are easier ways for ## malware to pop up lots of windows or consume huge amounts of RAM. -## * The shim's primary purpose is to ensure that files aren't accidentally -## opened when trying to view the contents of a directory. We do not care if -## the exact directory that exists at launch time is the directory that gets -## opened when the user chooses to open it. Therefore replacement attacks -## involving swapping one directory with another or with a symlink to a -## directory are out of scope. +## * The frontend is now a confirmation-only dialog. The actual file manager +## launch is handled by the backend via D-Bus forwarding to pcmanfm-qt, +## which eliminates the TOCTOU vulnerability that existed with CLI +## invocation. Exit code 0 = user confirmed, exit code 1 = user cancelled. ## * Python version lower than 3.13.5 are out of scope. """ fm_shim_frontend.py - Prompts the user if they want to open one or more -directories in the default file manager. +directories in the default file manager. This is a confirmation-only dialog; +the backend handles the actual file manager launch via D-Bus forwarding. +Exit code 0 means user confirmed, exit code 1 means user cancelled. """ import sys import os import signal -import subprocess import urllib.parse from pathlib import Path from typing import NoReturn, cast from types import FrameType -from PyQt5.QtCore import QTimer, QObject, QEvent, Qt +from PyQt5.QtCore import QTimer, QObject, QEvent from PyQt5.QtGui import QFontDatabase from PyQt5.QtWidgets import ( QApplication, @@ -44,7 +43,6 @@ from PyQt5.QtWidgets import ( QWidget, QScrollBar, QLayout, - QMessageBox, ) @@ -207,132 +205,23 @@ class FmShimWindow(QDialog): self.show() - # pylint: disable=too-many-branches def open_dir_list(self) -> None: """ - Opens all specified directories using the default file manager. + User confirmed opening the listed directories. + Exit with code 0 to signal confirmation to the backend, which + handles the actual file manager launch via D-Bus forwarding. """ - try: - default_fm_desktop_file: str = subprocess.run( - ["xdg-mime", "query", "default", "inode/directory"], - check=True, - capture_output=True, - encoding="utf-8", - ).stdout.strip() - except Exception: - print( - "ERROR: Unable to get default file manager!", - file=sys.stderr, - ) - sys.exit(1) - - ## Expect a plain desktop filename, not a path. - if ( - default_fm_desktop_file == "" - or "/" in default_fm_desktop_file - or not default_fm_desktop_file.endswith(".desktop") - ): - print( - "ERROR: Invalid default file manager desktop file name!", - file=sys.stderr, - ) - sys.exit(1) - - search_dir_list: list[str] = [] - - ## "" rather than None is intentional here, as - ## os.environ["XDG_DATA_HOME"] may be "", not None. - xdg_data_home: str = "" - try: - xdg_data_home = os.environ["XDG_DATA_HOME"] - except KeyError: - pass - if xdg_data_home == "": - home_env_var = "" - try: - home_env_var = os.environ["HOME"] - except KeyError: - pass - if home_env_var == "": - print( - "ERROR: Both the XDG_DATA_HOME and HOME environment " - + "variables are either undefined or empty!", - file=sys.stderr - ) - sys.exit(1) - xdg_data_home = home_env_var + "/.local/share" - if not xdg_data_home.startswith("/"): - print( - "ERROR: XDG_DATA_HOME is not an absolute path!", - file=sys.stderr - ) - sys.exit(1) - search_dir_list.append(xdg_data_home) - - ## Again, using "" rather than None is intentional here. - xdg_data_dirs: str = "" - try: - xdg_data_dirs = os.environ["XDG_DATA_DIRS"] - except Exception: - pass - xdg_data_dir_list = [p for p in xdg_data_dirs.split(":") if p != ""] - if len(xdg_data_dir_list) == 0: - xdg_data_dir_list = ["/usr/local/share", "/usr/share"] - if not all(x.startswith("/") for x in xdg_data_dir_list): - print( - "ERROR: XDG_DATA_DIRS contains non-absolute paths!", - file=sys.stderr, - ) - sys.exit(1) - search_dir_list.extend(xdg_data_dir_list) - - found_desktop_file: bool = False - for search_dir in search_dir_list: - default_fm_path: Path = Path( - search_dir + "/applications/" + default_fm_desktop_file - ) - if default_fm_path.is_file(): - found_desktop_file = True - break - if not found_desktop_file: - print( - "ERROR: Cannot find default file manager's desktop file!", - file=sys.stderr, - ) - sys.exit(1) - - for target_dir in self.dir_list: - if target_dir.is_dir(): - subprocess.run( - ["gio", "launch", str(default_fm_path), str(target_dir)], - check=False, - ) - else: - QMessageBox.warning( - self, - "Open Directories", - Qt.convertFromPlainText( - "ERROR: The following path was no longer a dir when " - + "checked before opening:\n" - + "\n" - + f"{target_dir}\n" - + "\n" - + "This may be the result of an attempted attack. No " - + "further directories will be opened." - ) - ) - sys.exit(1) - sys.exit(0) @staticmethod def exit_app() -> None: """ - Exits the application. + User cancelled. Exit with code 1 to signal cancellation to the + backend. """ - sys.exit(0) + sys.exit(1) # pylint: disable=unused-argument diff --git a/usr/share/dbus-1/services/org.freedesktop.FileManager1.service#security-misc-shared b/usr/share/dbus-1/services/org.freedesktop.FileManager1.service#security-misc-shared new file mode 100644 index 00000000..542702a2 --- /dev/null +++ b/usr/share/dbus-1/services/org.freedesktop.FileManager1.service#security-misc-shared @@ -0,0 +1,14 @@ +## Copyright (C) 2026 - 2026 ENCRYPTED SUPPORT LLC +## See the file COPYING for copying conditions. + +## D-Bus service activation file for the fm-shim backend. +## When an application calls org.freedesktop.FileManager1 methods +## (e.g. ShowFolders) and no process owns the name, D-Bus will +## activate this service via the systemd user unit. +## +## https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233 + +[D-BUS Service] +Name=org.freedesktop.FileManager1 +Exec=/usr/bin/fm-shim-backend +SystemdService=fm-shim.service diff --git a/usr/src/security-misc/fm-shim-backend.c#security-misc-shared b/usr/src/security-misc/fm-shim-backend.c#security-misc-shared index ae76b4f5..840480aa 100644 --- a/usr/src/security-misc/fm-shim-backend.c#security-misc-shared +++ b/usr/src/security-misc/fm-shim-backend.c#security-misc-shared @@ -1,13 +1,26 @@ /* - * Copyright (C) 2026 - 2026 ENCRYPTED SUPPORT LLC * See the file COPYING for copying conditions. */ /* * This is the backend half of an org.freedesktop.FileManager1 D-Bus interface - * handler. It catches method calls intended for that interface, and executes - * the frontend with arguments from the calls. This is intended to work around - * issues with PCManFM-Qt's D-Bus handling. + * handler. It claims the org.freedesktop.FileManager1 name, shows a + * confirmation dialog via the frontend, and then forwards confirmed requests + * to pcmanfm-qt via D-Bus (rather than CLI invocation), avoiding the TOCTOU + * vulnerability described in: + * https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233/39 + * + * The approach follows marmarek's suggestion: + * https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233/40 + * + * The shim acts as a "smarter D-Bus activation proxy": + * 1. Claims org.freedesktop.FileManager1 + * 2. On request: shows confirmation dialog (frontend) + * 3. If confirmed: releases name, starts pcmanfm-qt --daemon-mode, + * waits for it to claim the name, forwards the request via D-Bus + * 4. Re-requests the name (queued behind pcmanfm-qt) + * 5. When pcmanfm-qt exits, the shim becomes primary owner again * * Notes for reviewers: * @@ -17,10 +30,13 @@ * Attackers that can modify its environment via D-Bus can set * LD_LIBRARY_PATH in the user manager, which cannot be meaningfully * defended against. - * - The program intentionally return a "success" D-Bus reply even if no + * - The program intentionally returns a "success" D-Bus reply even if no * frontend popup is shown. The org.freedesktop.FileManager1 spec does not * specify what errors (if any) are safe to return, so always returning * empty response messages is the safest option. + * - Using D-Bus ShowFolders to communicate with pcmanfm-qt eliminates the + * TOCTOU vulnerability that existed with CLI invocation. pcmanfm-qt + * handles ShowFolders correctly (opens folders, does not open files). */ #include @@ -43,25 +59,42 @@ #define MAX_OPEN_PATHS 256 #define MAX_URI_LEN 4096 +/* Maximum time to wait for pcmanfm-qt to claim the D-Bus name (microseconds) */ +#define PCMANFM_WAIT_TIMEOUT_US 10000000 + +/* Polling interval when waiting for pcmanfm-qt (microseconds) */ +#define PCMANFM_POLL_INTERVAL_US 100000 + DBusError error_data = { 0 }; DBusConnection *dbus_conn = NULL; -void launch_frontend_process(const char *mode_opt, char **uri_list, - size_t uri_list_len) { - /* - * Most of the code here is inspired heavily by the qubes-gui-runuser - * function 'augment_pam_env_with_systemd_env()'. We need to get systemd's - * current set of environment variables to expose those to the frontend - * process, otherwise we won't know what display server to use and may run - * into theming issues. - * - * TODO: The environment in Non-Qubes-Whonix's user manager has some - * worrying discrepancies when compared to the environment of bash running - * in a qterminal window. In particular, 'PATH' is different. We may need to - * add a 'dbus-update-activation-environment --systemd --all' call to - * /usr/libexec/desktop-config-dist/start-lxqt-session. - */ +/* + * Free a NULL-terminated array of strdup'd strings. + */ +void free_env_array(char **env_arr) { + size_t i = 0; + + if (env_arr == NULL) { + return; + } + for (i = 0; env_arr[i] != NULL; i++) { + free(env_arr[i]); + } + free(env_arr); +} +/* + * Fetch the current environment from systemd's user manager via D-Bus. + * Returns a NULL-terminated array of strdup'd "KEY=VALUE" strings. + * The caller must free the result with free_env_array(). + * Returns NULL on error. + * + * This is inspired by qubes-gui-runuser's augment_pam_env_with_systemd_env(). + * We fetch the environment on each request so that environment updates + * (e.g. from dbus-update-activation-environment --systemd --all) are + * picked up without restarting the service. + */ +char **get_systemd_environment(void) { DBusMessage *env_request = NULL; const char *systemd_manager_str = "org.freedesktop.systemd1.Manager"; const char *environment_str = "Environment"; @@ -73,20 +106,18 @@ void launch_frontend_process(const char *mode_opt, char **uri_list, char *inner_iter_typesig = NULL; DBusMessageIter reply_arr_iter = { 0 }; const char *env_val = NULL; - const char **env_arr = NULL; + char **env_arr = NULL; size_t env_arr_len = 0; - const char **arg_arr = NULL; int current_type = 0; - size_t uri_list_idx = 0; - pid_t fork_pid = 0; + char *env_val_copy = NULL; env_request = dbus_message_new_method_call("org.freedesktop.systemd1", "/org/freedesktop/systemd1", "org.freedesktop.DBus.Properties", "Get"); if (env_request == NULL) { - warnx("launch_frontend_process: Failed to create D-Bus method call object!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Failed to create D-Bus method call object!"); + goto env_cleanup; } ret = dbus_message_append_args(env_request, @@ -94,90 +125,114 @@ void launch_frontend_process(const char *mode_opt, char **uri_list, DBUS_TYPE_STRING, &environment_str, DBUS_TYPE_INVALID); if (ret == FALSE) { - warnx("launch_frontend_process: Failed to append arguments to D-Bus method call object!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Failed to append arguments to D-Bus method call object!"); + goto env_cleanup; } env_reply = dbus_connection_send_with_reply_and_block(dbus_conn, env_request, 500, &error_data); if (env_reply == NULL) { - warnx("launch_frontend_process: Failed to request environment data from system! Error name: '%s'. Error contents: '%s'.", + warnx("get_systemd_environment: Failed to request environment data from system! Error name: '%s'. Error contents: '%s'.", error_data.name ? error_data.name : "(null)", error_data.message ? error_data.message : "(null)"); dbus_error_free(&error_data); - goto dbus_cleanup; + goto env_cleanup; } reply_type = dbus_message_get_type(env_reply); if (reply_type != DBUS_MESSAGE_TYPE_METHOD_RETURN) { - warnx("launch_frontend_process: Did not get method call return object from systemd!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Did not get method call return object from systemd!"); + goto env_cleanup; } ret = dbus_message_iter_init(env_reply, &reply_iter); if (ret == FALSE) { - warnx("launch_frontend_process: systemd returned an empty method call return object!"); - goto dbus_cleanup; + warnx("get_systemd_environment: systemd returned an empty method call return object!"); + goto env_cleanup; } if (dbus_message_iter_get_arg_type(&reply_iter) != DBUS_TYPE_VARIANT) { - warnx("launch_frontend_process: systemd did not return a variant object!"); - goto dbus_cleanup; + warnx("get_systemd_environment: systemd did not return a variant object!"); + goto env_cleanup; } dbus_message_iter_recurse(&reply_iter, &reply_inner_iter); inner_iter_typesig = dbus_message_iter_get_signature(&reply_inner_iter); if (inner_iter_typesig == NULL) { - err(1, "launch_frontend_process: Failed to allocate memory for type signature!"); + err(1, "get_systemd_environment: Failed to allocate memory for type signature!"); } if (strcmp(inner_iter_typesig, "as") != 0) { - warnx("launch_frontend_process: Variant object from systemd does not contain a string array!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Variant object from systemd does not contain a string array!"); + goto env_cleanup; } if (dbus_message_iter_get_arg_type(&reply_inner_iter) != DBUS_TYPE_ARRAY) { - warnx("launch_frontend_process: Variant object from systemd reported itself as a string array, but is not an array!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Variant object from systemd reported itself as a string array, but is not an array!"); + goto env_cleanup; } dbus_message_iter_recurse(&reply_inner_iter, &reply_arr_iter); while ((current_type = dbus_message_iter_get_arg_type(&reply_arr_iter)) != DBUS_TYPE_INVALID) { if (current_type != DBUS_TYPE_STRING) { - warnx("launch_frontend_process: Non-string item found in string array!"); - goto dbus_cleanup; + warnx("get_systemd_environment: Non-string item found in string array!"); + free_env_array(env_arr); + env_arr = NULL; + goto env_cleanup; } dbus_message_iter_get_basic(&reply_arr_iter, &env_val); + env_val_copy = strdup(env_val); + if (env_val_copy == NULL) { + err(1, "get_systemd_environment: Failed to strdup environment variable!"); + } env_arr_len++; - env_arr = reallocarray(env_arr, env_arr_len, sizeof(const char *)); + env_arr = reallocarray(env_arr, env_arr_len, sizeof(char *)); if (env_arr == NULL) { - err(1, "launch_frontend_process: Failed to allocate memory for environment array!"); + err(1, "get_systemd_environment: Failed to allocate memory for environment array!"); } - env_arr[env_arr_len - 1] = env_val; + env_arr[env_arr_len - 1] = env_val_copy; dbus_message_iter_next(&reply_arr_iter); } - /* execve requires the environment array to be null terminated */ + /* NULL-terminate the array (required by execve) */ env_arr_len++; - env_arr = reallocarray(env_arr, env_arr_len, sizeof(const char *)); + env_arr = reallocarray(env_arr, env_arr_len, sizeof(char *)); if (env_arr == NULL) { - err(1, "launch_frontend_process: Failed to allocate memory for environment array!"); + err(1, "get_systemd_environment: Failed to allocate memory for environment array!"); } env_arr[env_arr_len - 1] = NULL; - /* - * This should be impossible since uri_list_len is originally a signed - * integer in handle_dbus_method_call and is then cast to a size_t, but we - * check for this anyway. - */ +env_cleanup: + if (inner_iter_typesig != NULL) { + dbus_free(inner_iter_typesig); + } + if (env_reply != NULL) { + dbus_message_unref(env_reply); + } + if (env_request != NULL) { + dbus_message_unref(env_request); + } + return env_arr; +} + +/* + * Fork and exec the frontend confirmation dialog. + * Returns the child PID on success, or -1 on failure. + * The caller should waitpid() on the returned PID. + */ +pid_t launch_frontend(const char *mode_opt, char **uri_list, + size_t uri_list_len, char **env_arr) { + const char **arg_arr = NULL; + size_t uri_list_idx = 0; + pid_t fork_pid = 0; + if (uri_list_len > SIZE_MAX - 4) { - warnx("launch_frontend_process: URI list too large!"); - goto dbus_cleanup; + warnx("launch_frontend: URI list too large!"); + return -1; } - /* Now that we have the environment array, we can build the arg array. */ arg_arr = reallocarray(arg_arr, uri_list_len + 4, sizeof(const char *)); if (arg_arr == NULL) { - err(1, "launch_frontend_process: Failed to allocate memory for argument array!"); + err(1, "launch_frontend: Failed to allocate memory for argument array!"); } arg_arr[0] = "/usr/bin/fm-shim-frontend"; arg_arr[1] = mode_opt; @@ -187,47 +242,133 @@ void launch_frontend_process(const char *mode_opt, char **uri_list, } arg_arr[uri_list_len + 3] = NULL; - /* Fork and exec the frontend */ fork_pid = fork(); if (fork_pid == -1) { - err(1, "launch_frontend_process: fork failed"); + warn("launch_frontend: fork failed"); + free(arg_arr); + return -1; } else if (fork_pid == 0) { /* child */ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-qual" if (execve(arg_arr[0], (char **)arg_arr, (char **)env_arr) == -1) { - warn("launch_frontend_process: Could not exec '/usr/bin/fm-shim-frontend'"); + warn("launch_frontend: Could not exec '/usr/bin/fm-shim-frontend'"); _exit(1); } #pragma GCC diagnostic pop } /* parent */ + free(arg_arr); + return fork_pid; +} -dbus_cleanup: - /* Don't free the elements of arg_arr, they are not freeable. */ - if (arg_arr != NULL) { - free(arg_arr); +/* + * Start pcmanfm-qt in daemon mode using double-fork to avoid zombies. + * The grandchild process is orphaned and adopted by init. + */ +void launch_pcmanfm_qt_daemon(char **env_arr) { + pid_t pid1 = 0; + pid_t pid2 = 0; + const char *argv[] = { + "/usr/bin/pcmanfm-qt", + "--daemon-mode", + NULL + }; + + pid1 = fork(); + if (pid1 == -1) { + warn("launch_pcmanfm_qt_daemon: first fork failed"); + return; + } + if (pid1 == 0) { + /* first child: fork again and exit immediately */ + pid2 = fork(); + if (pid2 == -1) { + _exit(1); + } + if (pid2 == 0) { + /* grandchild: exec pcmanfm-qt */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-qual" + execve(argv[0], (char **)argv, (char **)env_arr); +#pragma GCC diagnostic pop + /* If execve returns, it failed */ + _exit(1); + } + /* first child exits so grandchild is orphaned */ + _exit(0); } - /* Ditto for env_arr */ - if (env_arr != NULL) { - free(env_arr); + /* parent: wait for first child (which exits immediately) */ + waitpid(pid1, NULL, 0); +} + +/* + * Poll until org.freedesktop.FileManager1 has an owner on D-Bus, + * or until the timeout expires. + * Returns true if the name has an owner, false on timeout. + */ +bool wait_for_dbus_name_owner(void) { + int elapsed_us = 0; + + while (elapsed_us < PCMANFM_WAIT_TIMEOUT_US) { + if (dbus_bus_name_has_owner(dbus_conn, + "org.freedesktop.FileManager1", NULL) == TRUE) { + return true; + } + usleep((unsigned int)PCMANFM_POLL_INTERVAL_US); + elapsed_us += PCMANFM_POLL_INTERVAL_US; } - if (inner_iter_typesig != NULL) { - dbus_free(inner_iter_typesig); + return false; +} + +/* + * Forward a ShowFolders/ShowItems/ShowItemProperties request to + * whichever process now owns org.freedesktop.FileManager1 (pcmanfm-qt). + * This uses D-Bus method call rather than CLI invocation, which + * eliminates the TOCTOU vulnerability. + */ +void forward_dbus_request(const char *method_name, char **uri_list, + int uri_list_len, const char *startup_id) { + DBusMessage *fwd_msg = NULL; + dbus_bool_t ret = FALSE; + const char *sid = NULL; + + sid = (startup_id != NULL) ? startup_id : ""; + + fwd_msg = dbus_message_new_method_call( + "org.freedesktop.FileManager1", + "/org/freedesktop/FileManager1", + "org.freedesktop.FileManager1", + method_name); + if (fwd_msg == NULL) { + warnx("forward_dbus_request: Failed to create D-Bus method call!"); + return; } - if (env_reply != NULL) { - dbus_message_unref(env_reply); + + ret = dbus_message_append_args(fwd_msg, + DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &uri_list, uri_list_len, + DBUS_TYPE_STRING, &sid, + DBUS_TYPE_INVALID); + if (ret == FALSE) { + warnx("forward_dbus_request: Failed to append arguments!"); + dbus_message_unref(fwd_msg); + return; } - if (env_request != NULL) { - dbus_message_unref(env_request); + + /* Fire-and-forget: we don't need a reply */ + dbus_message_set_no_reply(fwd_msg, TRUE); + if (dbus_connection_send(dbus_conn, fwd_msg, NULL) == FALSE) { + warnx("forward_dbus_request: Failed to send method call!"); } + dbus_connection_flush(dbus_conn); + dbus_message_unref(fwd_msg); } /* * All three of the method calls we handle take the same arguments, and we do - * the hard work in the frontend, so we can use the same function here to - * handle all the calls. + * the hard work in the frontend and via D-Bus forwarding, so we can use the + * same function here to handle all the calls. */ void handle_dbus_method_call(DBusMessage *dbus_msg, const char *method_call_name, const char *mode_opt) { @@ -238,7 +379,11 @@ void handle_dbus_method_call(DBusMessage *dbus_msg, const char *startup_id = NULL; DBusMessage *method_return = NULL; size_t uri_list_idx = 0; - bool do_launch_frontend = false; + bool do_process = false; + char **env_arr = NULL; + pid_t frontend_pid = -1; + int frontend_status = 0; + int dbus_name_re_request = 0; did_extract_args = dbus_message_get_args(dbus_msg, &error_data, @@ -277,11 +422,11 @@ void handle_dbus_method_call(DBusMessage *dbus_msg, } } - do_launch_frontend = true; + do_process = true; method_send_reply: if (dbus_message_get_no_reply(dbus_msg) == TRUE) { - goto launch_frontend; + goto process_request; } method_return = dbus_message_new_method_return(dbus_msg); @@ -292,18 +437,82 @@ method_send_reply: err(1, "handle_dbus_method_call: Could not queue method return!"); } /* - * We could probably get away with omitting the flush, but browsers will - * attempt to launch the file manager by themselves if they don't get a - * return message quickly enough, so push this out as quickly as possible. + * Push the reply out immediately so the calling application (e.g. browser) + * does not attempt to launch its own file manager due to timeout. */ dbus_connection_flush(dbus_conn); -launch_frontend: - if (do_launch_frontend) { - launch_frontend_process(mode_opt, uri_list, uri_list_len); +process_request: + if (!do_process) { + goto method_cleanup; + } + + /* Fetch current environment from systemd's user manager */ + env_arr = get_systemd_environment(); + if (env_arr == NULL) { + warnx("handle_dbus_method_call: Could not get environment from systemd, cannot process request."); + goto method_cleanup; + } + + /* Launch frontend confirmation dialog and wait for user response */ + frontend_pid = launch_frontend(mode_opt, uri_list, uri_list_len, env_arr); + if (frontend_pid <= 0) { + warnx("handle_dbus_method_call: Could not launch frontend."); + goto method_cleanup; + } + + if (waitpid(frontend_pid, &frontend_status, 0) == -1) { + warn("handle_dbus_method_call: waitpid for frontend failed"); + goto method_cleanup; + } + + if (!WIFEXITED(frontend_status) || WEXITSTATUS(frontend_status) != 0) { + /* User cancelled or frontend encountered an error */ + goto method_cleanup; + } + + /* + * User confirmed. Forward the request to pcmanfm-qt via D-Bus. + * + * 1. Release our D-Bus name so pcmanfm-qt can claim it + * 2. Start pcmanfm-qt --daemon-mode (double-fork to avoid zombies) + * 3. Wait for pcmanfm-qt to claim the name + * 4. Forward the original request via D-Bus + * 5. Re-request the name (we'll be queued behind pcmanfm-qt) + */ + + dbus_bus_release_name(dbus_conn, "org.freedesktop.FileManager1", + &error_data); + dbus_error_free(&error_data); + + launch_pcmanfm_qt_daemon(env_arr); + + if (!wait_for_dbus_name_owner()) { + warnx("handle_dbus_method_call: pcmanfm-qt did not claim D-Bus name within timeout."); + /* Re-claim the name ourselves since pcmanfm-qt failed to start */ + dbus_bus_request_name(dbus_conn, "org.freedesktop.FileManager1", + DBUS_NAME_FLAG_REPLACE_EXISTING, &error_data); + dbus_error_free(&error_data); + goto method_cleanup; + } + + forward_dbus_request(method_call_name, uri_list, uri_list_len_int, + startup_id); + + /* Re-request the name; we'll be queued behind pcmanfm-qt */ + dbus_name_re_request = dbus_bus_request_name(dbus_conn, + "org.freedesktop.FileManager1", 0, &error_data); + dbus_error_free(&error_data); + if (dbus_name_re_request == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { + printf("Re-acquired 'org.freedesktop.FileManager1' name.\n"); + } else { + printf("Queued behind pcmanfm-qt for 'org.freedesktop.FileManager1' name.\n"); } method_cleanup: + if (env_arr != NULL) { + free_env_array(env_arr); + } if (method_return != NULL) { dbus_message_unref(method_return); } @@ -316,9 +525,13 @@ int main(void) { struct sigaction sigchld_act = { 0 }; int dbus_name_request_rslt = 0; - sigchld_act.sa_handler = SIG_IGN; + /* + * Use default SIGCHLD handling so we can waitpid() for the frontend + * process. pcmanfm-qt is started via double-fork to avoid zombies. + */ + sigchld_act.sa_handler = SIG_DFL; if (sigaction(SIGCHLD, &sigchld_act, NULL) == -1) { - err(1, "Failed to enable automatic child process reaping"); + err(1, "Failed to set SIGCHLD handler"); } dbus_error_init(&error_data); @@ -346,13 +559,17 @@ int main(void) { sd_notify(0, "STATUS=Running"); break; case DBUS_REQUEST_NAME_REPLY_IN_QUEUE: - warnx("Requested 'org.freedesktop.FileManager1' name from D-Bus, but was placed in queue."); - warnx("This may be a security risk! Please report this bug!"); /* - * sd_notify(0, "READY=1") is intentionally omitted here, so that - * systemcheck will show a warning about the system not being ready + * Another process (e.g. pcmanfm-qt in daemon mode) already owns the + * name. This is normal and not a security issue. We stay running + * and will automatically become the primary owner when the current + * owner exits. + * + * We send READY=1 so that systemd does not time out the service. */ - sd_notify(0, "STATUS=Running, security issue detected"); + printf("Another process already owns 'org.freedesktop.FileManager1'. Queued as backup handler.\n"); + sd_notify(0, "READY=1"); + sd_notify(0, "STATUS=Queued, another file manager handler is active"); break; case DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER: warnx("Possible bug, we already own 'org.freedesktop.FileManager1' name?"); From 302b1aecadf03a74c2e31260b406ae52f1d15e2a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 10:55:09 +0000 Subject: [PATCH 2/2] =?UTF-8?q?Remove=20env=20sync=20autostart=20=E2=80=94?= =?UTF-8?q?=20Qubes=20env=20mismatch=20belongs=20upstream?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale systemd user manager environment on Qubes OS is a Qubes bug (QubesOS/qubes-issues), not a security-misc bug. Working around it here is fixing the issue in the wrong place. The D-Bus forwarding approach already eliminates the dependency on xdg-mime (which was the component affected by stale XDG_DATA_DIRS), so the env mismatch no longer causes Catfish to open. https://claude.ai/code/session_01SAkV8U9vft3vzZAyJfzXGE --- debian/security-misc-shared.install | 1 - ...dbus-env-sync.desktop#security-misc-shared | 23 ------------------- 2 files changed, 24 deletions(-) delete mode 100644 etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared diff --git a/debian/security-misc-shared.install b/debian/security-misc-shared.install index f63ef079..32672791 100755 --- a/debian/security-misc-shared.install +++ b/debian/security-misc-shared.install @@ -6,7 +6,6 @@ ## This file was generated using 'genmkfile debinstfile'. etc/apparmor.d/tunables/home.d/security-misc#security-misc-shared => /etc/apparmor.d/tunables/home.d/security-misc -etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared => /etc/xdg/autostart/security-misc-dbus-env-sync.desktop etc/apt/apt.conf.d/40error-on-any#security-misc-shared => /etc/apt/apt.conf.d/40error-on-any etc/apt/apt.conf.d/40sandbox#security-misc-shared => /etc/apt/apt.conf.d/40sandbox etc/default/grub.d/40_cpu_mitigations.cfg#security-misc-shared => /etc/default/grub.d/40_cpu_mitigations.cfg diff --git a/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared b/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared deleted file mode 100644 index f2532ac6..00000000 --- a/etc/xdg/autostart/security-misc-dbus-env-sync.desktop#security-misc-shared +++ /dev/null @@ -1,23 +0,0 @@ -## Copyright (C) 2026 - 2026 ENCRYPTED SUPPORT LLC -## See the file COPYING for copying conditions. - -## Synchronize session environment variables to the D-Bus activation -## environment and systemd's user manager. This ensures that services -## started by systemd or D-Bus activation (such as fm-shim) have -## access to the correct DISPLAY, XDG_DATA_DIRS, and other session -## variables. -## -## This is necessary on Qubes OS where the session setup does not -## automatically sync the environment, causing services to operate -## with stale or missing variables. Standard desktop environments -## (GNOME, KDE) perform this sync as part of their session startup. -## -## https://forums.whonix.org/t/is-catfish-the-new-default-in-the-qubes-domain-open-file-manager-quick-widget-for-whonix/20233 - -[Desktop Entry] -Type=Application -Name=D-Bus Environment Sync -Comment=Sync session environment to D-Bus and systemd user manager -Exec=/bin/sh -c 'dbus-update-activation-environment --systemd --all' -StartupNotify=false -NoDisplay=true