Improve fm-shim-backend security and D-Bus error handling by assisted-by-ai · Pull Request #358 · Kicksecure/security-misc

assisted-by-ai · 2026-04-01T14:39:49Z

Summary

This PR enhances the security and robustness of the fm-shim-backend service by implementing file descriptor cleanup in child processes, improving D-Bus error handling, and adding systemd hardening options.

Key Changes

File Descriptor Management:

Added automatic cleanup of inherited file descriptors (except stdin/stdout/stderr) in child processes by scanning /proc/self/fd before executing the frontend process
Prevents accidental leakage of sensitive file descriptors to child processes

D-Bus Improvements:

Changed D-Bus name request to use DBUS_NAME_FLAG_DO_NOT_QUEUE flag, preventing the service from queuing if the name is already taken
Converted queued name request from a warning to a fatal error, treating it as a security issue
Added proper D-Bus error replies for invalid object paths, interfaces, and methods instead of silently ignoring them
Added cleanup of D-Bus connection and error data structures on exit

Systemd Hardening:

Added security-focused systemd service options:
- NoNewPrivileges=yes - Prevents privilege escalation
- ProtectSystem=strict - Read-only access to system directories
- ProtectHome=read-only - Read-only access to home directory
- PrivateTmp=yes - Private temporary directory
- RestrictNamespaces=yes - Restrict namespace creation
- RestrictRealtime=yes - Disable real-time scheduling
- MemoryDenyWriteExecute=yes - Prevent executable memory allocation
- LockPersonality=yes - Lock process personality

Implementation Details

Uses opendir() and readdir() to enumerate file descriptors safely
Validates file descriptor numbers using strtol() with proper error checking
Maintains compatibility by preserving the directory file descriptor during enumeration
D-Bus error replies are only sent when the client expects a reply

https://claude.ai/code/session_01CgrrFhhVWtqtfhvJdEcBWm

…ene, systemd sandboxing, and cleanup - Add DBUS_NAME_FLAG_DO_NOT_QUEUE and terminate on IN_QUEUE to prevent silent name takeover after another owner exits - Send proper D-Bus error replies for unrecognized methods, wrong object paths, and wrong interfaces instead of leaving callers hanging - Close inherited file descriptors in child process before execve to avoid leaking the D-Bus socket to the frontend - Add systemd sandboxing directives (NoNewPrivileges, ProtectSystem, ProtectHome, PrivateTmp, etc.) to the user service - Add explicit dbus_connection_unref/dbus_error_free on main loop exit https://claude.ai/code/session_01CgrrFhhVWtqtfhvJdEcBWm

ArrayBolt3

Integrated the useful parts of this into ArrayBolt3@af4f229. Notes below.

ArrayBolt3 · 2026-04-05T22:35:56Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

-    DBUS_NAME_FLAG_REPLACE_EXISTING,
+    DBUS_NAME_FLAG_REPLACE_EXISTING | DBUS_NAME_FLAG_DO_NOT_QUEUE,


Again, no, this is not what we want to do.

ArrayBolt3 · 2026-04-05T22:36:13Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

-    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
-     */
-    sd_notify(0, "STATUS=Running, security issue detected");
+    errx(1, "Requested 'org.freedesktop.FileManager1' name from D-Bus, but was placed in queue. This is a security risk! Please report this bug!");


No, this is not what we want to do. This makes the security risk worse.

ArrayBolt3 · 2026-04-05T22:43:17Z

usr/lib/systemd/user/fm-shim.service#security-misc-shared

+NoNewPrivileges=yes
+ProtectSystem=strict
+ProtectHome=read-only
+PrivateTmp=yes
+RestrictNamespaces=yes
+RestrictRealtime=yes
+MemoryDenyWriteExecute=yes
+LockPersonality=yes


I believe all of these restrictions will be inherited by the child process (the file manager) and any processes it launches, which probably will lead to strange behavior. Even MemoryDenyWriteExecute=yes might be problematic if the user launches a terminal from within the file manager, then launches a JIT-compiled applications from there.

ArrayBolt3 · 2026-04-05T22:56:02Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

+      if (dbus_message_get_no_reply(dbus_msg) == FALSE) {
+        DBusMessage *err_reply = dbus_message_new_error(dbus_msg,
+          DBUS_ERROR_UNKNOWN_OBJECT, "Unknown object path");
+        if (err_reply != NULL) {
+          dbus_connection_send(dbus_conn, err_reply, NULL);
+          dbus_message_unref(err_reply);
+        }
+      }


Accepted with some refactoring to avoid duplication.

ArrayBolt3 · 2026-04-05T22:56:13Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

+      if (dbus_message_get_no_reply(dbus_msg) == FALSE) {
+        DBusMessage *err_reply = dbus_message_new_error(dbus_msg,
+          DBUS_ERROR_UNKNOWN_INTERFACE, "Unknown interface");
+        if (err_reply != NULL) {
+          dbus_connection_send(dbus_conn, err_reply, NULL);
+          dbus_message_unref(err_reply);
+        }
+      }


Accepted with refactoring.

ArrayBolt3 · 2026-04-05T22:56:17Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

+      if (dbus_message_get_no_reply(dbus_msg) == FALSE) {
+        DBusMessage *err_reply = dbus_message_new_error(dbus_msg,
+          DBUS_ERROR_UNKNOWN_METHOD, "Unknown method");
+        if (err_reply != NULL) {
+          dbus_connection_send(dbus_conn, err_reply, NULL);
+          dbus_message_unref(err_reply);
+        }
+      }


Accepted with refactoring.

ArrayBolt3 · 2026-04-05T22:59:52Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

+
+  dbus_connection_unref(dbus_conn);
+  dbus_error_free(&error_data);


ArrayBolt3 · 2026-04-05T23:06:48Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

-    /* child */
+    /* child - close inherited fds (except stdin/stdout/stderr) */
+    DIR *fd_dir = opendir("/proc/self/fd");
+    if (fd_dir != NULL) {
+      int dir_fd = dirfd(fd_dir);
+      struct dirent *fd_entry;
+      while ((fd_entry = readdir(fd_dir)) != NULL) {
+        char *endptr = NULL;
+        long fd_num = strtol(fd_entry->d_name, &endptr, 10);
+        if (endptr == fd_entry->d_name || *endptr != '\0') {
+          continue;
+        }
+        if (fd_num >= 3 && fd_num != (long)dir_fd) {
+          close((int)fd_num);
+        }
+      }
+      closedir(fd_dir);
+    }


I guess this is safe since even if libdbus uses multiple threads in the background (which hopefully it doesn't but you never know!), fork() only gives us a single thread. libdbus should set CLOEXEC on its socket file descriptor from what I can tell, and we don't open anything else, so this shouldn't be strictly necessary, but I guess it's a useful hardening measure, so I'll accept the idea. Implementation-wise though, this should be using the close_range() system call, not manual iteration through things in /proc.

ArrayBolt3 · 2026-04-05T23:21:15Z

usr/src/security-misc/fm-shim-backend.c#security-misc-shared

 #include <unistd.h>
 #include <assert.h>
 #include <sys/wait.h>
+#include <dirent.h>


Unnecessary since I'm using close_range() instead.

ArrayBolt3 reviewed Apr 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve fm-shim-backend security and D-Bus error handling#358

Improve fm-shim-backend security and D-Bus error handling#358
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
assisted-by-ai:claude/security-review-fm-shim-8hki2

assisted-by-ai commented Apr 1, 2026

Uh oh!

ArrayBolt3 left a comment

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

ArrayBolt3 Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		DBUS_NAME_FLAG_REPLACE_EXISTING,
		DBUS_NAME_FLAG_REPLACE_EXISTING \| DBUS_NAME_FLAG_DO_NOT_QUEUE,


		dbus_connection_unref(dbus_conn);
		dbus_error_free(&error_data);

Conversation

assisted-by-ai commented Apr 1, 2026

Summary

Key Changes

Implementation Details

Uh oh!

ArrayBolt3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants