From 624285d6320711d7372c3fd24a994c84e48b127a Mon Sep 17 00:00:00 2001 From: Devon Kirk Date: Thu, 18 Jun 2026 16:19:38 -0400 Subject: [PATCH] fix: restrict D-Bus setCurrentReference to navigation references only The org.xiphos.remote D-Bus service is registered on the session bus with G_BUS_NAME_OWNER_FLAGS_NONE, so any peer on the user's session bus can call setCurrentReference(). The handler passed the untrusted reference straight to main_url_handler() with clicked=TRUE, which not only navigates but also honours actions that reach the local filesystem or spawn external programs: - "showStudypad" loads an arbitrary file into the StudyPad editor; - "showImage" hands a path to an external viewer (xdg-open, etc.); - any URL matching none of the known schemes falls through to xiphos_open_default(), which opens an arbitrary URI/file in its default handler. A D-Bus policy file cannot meaningfully fix this: the session bus is per-user, so all peers share the caller's UID and a default-deny policy would simply disable the remote-control feature for everyone. Instead, validate the reference at the IPC boundary and accept only navigation references (sword://, bible://, passagestudy.jsp, xiphos.url) while rejecting the local-file actions and the open-anything fall-through. clicked=TRUE is preserved so legitimate navigation still works. This supersedes the earlier, non-functional approach of shipping an org.xiphos.remote.conf whose "mandatory" allow rule overrode its "default" deny (and which was never installed by the build). --- src/gtk/ipc.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/gtk/ipc.c b/src/gtk/ipc.c index a9de8bcc8..13217d20c 100644 --- a/src/gtk/ipc.c +++ b/src/gtk/ipc.c @@ -206,12 +206,34 @@ gboolean ipc_object_set_current_reference(IpcObject *obj, gchar *reference, GError **error) { - //easy route + /* + * Security: this method is reachable by any peer on the D-Bus session + * bus (the name is owned with G_BUS_NAME_OWNER_FLAGS_NONE), so the + * reference is untrusted input. main_url_handler() is called with + * clicked=TRUE, which is required for navigation to actually happen, + * but with clicked=TRUE it also honours actions that touch the local + * filesystem or launch external programs: + * - "showStudypad" loads an arbitrary file into the StudyPad editor, + * - "showImage" hands a path to an external viewer (xdg-open, ...), + * - any URL that matches none of the known schemes falls through to + * xiphos_open_default(), opening an arbitrary URI/file. + * Restrict remote callers to navigation references only: the URL must + * use one of the known navigation schemes/markers and must not request + * a local-file action. Reject everything else (fail closed). + */ + if (!reference || + (!g_strstr_len(reference, -1, "sword://") && + !g_strstr_len(reference, -1, "bible://") && + !g_strstr_len(reference, -1, "passagestudy.jsp") && + !g_strstr_len(reference, -1, "xiphos.url")) || + g_strstr_len(reference, -1, "showStudypad") || + g_strstr_len(reference, -1, "showImage")) { + g_warning("ipc: rejected non-navigation reference: %s", + reference ? reference : "(null)"); + return FALSE; + } + main_url_handler((const gchar *)reference, TRUE); - //it should be done like this, probably - /* g_signal_emit(obj, - "navigate-requested", - reference); */ return TRUE; }