From ada6943879fa180399f0fd950ebf80566416bd6e Mon Sep 17 00:00:00 2001 From: Mavis Date: Tue, 28 Jul 2026 00:07:43 +0000 Subject: [PATCH 1/2] ev-print-operation: Use g_spawn_async with argv instead of cmdline string When launching the print previewer, the previous code built a command-line string with g_strdup_printf(), quoted the file paths with g_shell_quote(), and then fed that string to g_app_info_create_from_commandline(), which parses and re-splits the string. Although the file paths came from g_file_open_tmp() and the export job (so they are glib-controlled in practice), this pattern is fragile: any future change that lets a user-controlled value flow into print_settings_file or export->temp_file would re-introduce the same class of argument-splitter injection that was recently fixed in ev_spawn (commit 50052ea). Replace the cmdline-string approach with a direct g_spawn_async() call that uses an explicit argv[]. Each element is passed verbatim to the child, eliminating the parse/re-split step. The child inherits the parent's environment, so DISPLAY / Wayland socket are still propagated without an explicit GdkAppLaunchContext. This also removes the dependency on g_app_info_create_from_commandline() and the temporary GAppInfo / GdkAppLaunchContext objects. --- libview/ev-print-operation.c | 48 ++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/libview/ev-print-operation.c b/libview/ev-print-operation.c index 663026b1..dae5d450 100644 --- a/libview/ev-print-operation.c +++ b/libview/ev-print-operation.c @@ -851,33 +851,27 @@ export_print_done (EvPrintOperationExport *export) g_key_file_free (key_file); if (!error) { - GAppInfo *app; - GdkAppLaunchContext *ctx; - gchar *cmd; - gchar *quoted_filename; - gchar *quoted_settings_filename; - - quoted_filename = g_shell_quote (export->temp_file); - quoted_settings_filename = g_shell_quote (print_settings_file); - cmd = g_strdup_printf ("xreader-previewer --unlink-tempfile --print-settings %s %s", - quoted_settings_filename, quoted_filename); - - g_free (quoted_filename); - g_free (quoted_settings_filename); - - app = g_app_info_create_from_commandline (cmd, NULL, 0, &error); - - if (app != NULL) { - ctx = gdk_display_get_app_launch_context (gtk_widget_get_display (GTK_WIDGET (export->parent_window))); - gdk_app_launch_context_set_screen (ctx, gtk_window_get_screen (export->parent_window)); - - g_app_info_launch (app, NULL, G_APP_LAUNCH_CONTEXT (ctx), &error); - - g_object_unref (app); - g_object_unref (ctx); - } - - g_free (cmd); + /* Use g_spawn_async with an explicit argv array rather than + * g_app_info_create_from_commandline(). The latter parses a + * command-line string and re-splits it, which is fragile if any + * argument contains whitespace or quoting-special characters. + * With argv, each element is passed verbatim to the child. + * + * The spawned previewer inherits the parent's environment + * (including DISPLAY/Wayland socket), so no explicit launch + * context is required. + */ + gchar *argv[] = { + (gchar *) "xreader-previewer", + (gchar *) "--unlink-tempfile", + (gchar *) "--print-settings", + print_settings_file, + export->temp_file, + NULL + }; + + g_spawn_async (NULL, argv, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, NULL, &error); } if (error) { From 4855f217a27053a43afd8b2004af33dc7a1f10bb Mon Sep 17 00:00:00 2001 From: Mavis Date: Tue, 28 Jul 2026 00:08:36 +0000 Subject: [PATCH 2/2] main: Use g_spawn_async with argv when re-spawning xreader-previewer The launch_previewer() helper in shell/main.c was building a command-line string with g_strdup_printf() / g_shell_quote() and then feeding it to g_app_info_create_from_commandline(). This is the same fragile pattern that was just replaced in ev-print-operation: the string is parsed and re-split by the helper, so any character that is meaningful to that parser (quotes, backslashes, whitespace) can change the meaning of the resulting argv. The two user-influenced arguments (print_settings, file_arguments) come from xreader's own command line, so the practical risk is limited to whatever the user / launching environment typed -- but the structural issue is identical to the print previewer case. Replace the cmdline-string path with a direct g_spawn_async() call that uses an explicit argv[] array built from the original option values, with no quoting or splitting involved. --- shell/main.c | 63 +++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/shell/main.c b/shell/main.c index bc5f96fc..0562cba5 100644 --- a/shell/main.c +++ b/shell/main.c @@ -80,56 +80,53 @@ static const GOptionEntry goption_options[] = static gboolean launch_previewer (void) { - GString *cmd_str; - gchar *cmd; + gchar **argv; + gint argc; gboolean retval = FALSE; GError *error = NULL; /* Rebuild the command line, ignoring options * not supported by the previewer and taking only - * the first path given + * the first path given. + * + * Use g_spawn_async() with an explicit argv[] array rather than + * g_app_info_create_from_commandline() so each argument is passed + * verbatim to the child. print_settings and file_arguments come from + * the xreader command line itself; even if g_shell_quote() is applied + * defensively, going through the commandline-string parser is fragile + * and mirrors the pattern recently patched in ev_spawn (50052ea). */ - cmd_str = g_string_new ("xreader-previewer"); - + argc = 1; + if (print_settings) argc += 2; + if (unlink_temp_file) argc += 1; + if (file_arguments) argc += 1; + argc += 1; /* trailing NULL */ + + argv = g_new0 (gchar *, argc); + argc = 0; + argv[argc++] = (gchar *) "xreader-previewer"; if (print_settings) { - gchar *quoted; - - quoted = g_shell_quote (print_settings); - g_string_append_printf (cmd_str, " --print-settings %s", quoted); - g_free (quoted); + argv[argc++] = (gchar *) "--print-settings"; + argv[argc++] = print_settings; } - if (unlink_temp_file) - g_string_append (cmd_str, " --unlink-tempfile"); - - if (file_arguments) { - gchar *quoted; - - quoted = g_shell_quote (file_arguments[0]); - g_string_append_printf (cmd_str, " %s", quoted); - g_free (quoted); + argv[argc++] = (gchar *) "--unlink-tempfile"; + if (file_arguments) + argv[argc++] = file_arguments[0]; + argv[argc] = NULL; + + if (g_spawn_async (NULL, argv, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, NULL, &error)) { + retval = TRUE; } - cmd = g_string_free (cmd_str, FALSE); - - if (!error) { - GAppInfo *app; - - app = g_app_info_create_from_commandline (cmd, NULL, 0, &error); - - if (app != NULL) { - retval = g_app_info_launch (app, NULL, NULL, &error); - g_object_unref (app); - } - } + g_free (argv); if (error) { g_warning ("Error launching previewer: %s\n", error->message); g_error_free (error); } - g_free (cmd); - return retval; }