From 6ab622d341e5e176ffe01118928474d5f239bb19 Mon Sep 17 00:00:00 2001 From: Michal Ruprich Date: Wed, 17 Jul 2024 15:55:24 +0200 Subject: [PATCH] Resolves: RHEL-14596 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe. --- ...02-Customize-permission-denied-error.patch | 3 +- ...shark-0018-sync-pipe-stderr-messages.patch | 439 ++++++++++++++++++ wireshark.spec | 6 +- 3 files changed, 445 insertions(+), 3 deletions(-) create mode 100644 wireshark-0018-sync-pipe-stderr-messages.patch diff --git a/wireshark-0002-Customize-permission-denied-error.patch b/wireshark-0002-Customize-permission-denied-error.patch index db8296f..e48549b 100644 --- a/wireshark-0002-Customize-permission-denied-error.patch +++ b/wireshark-0002-Customize-permission-denied-error.patch @@ -41,13 +41,12 @@ index 2f9d2cc..b18e47f 100644 int sync_pipe[2]; /* pipe used to send messages from child to parent */ int data_pipe[2]; /* pipe used to send data from child to parent */ #endif -@@ -1003,8 +1008,11 @@ sync_pipe_open_command(char** argv, int *data_read_fd, +@@ -1003,8 +1008,10 @@ sync_pipe_open_command(char** argv, int *data_read_fd, ws_close(sync_pipe[PIPE_READ]); ws_close(sync_pipe[PIPE_WRITE]); execv(argv[0], argv); - g_snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s", - argv[0], g_strerror(errno)); -+ execv(argv[0], (gpointer)argv); + if (errno == EPERM || errno == EACCES) + securitymsg = "\nAre you a member of the 'wireshark' group? Try running\n'usermod -a -G wireshark _your_username_' as root."; + g_snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s%s", diff --git a/wireshark-0018-sync-pipe-stderr-messages.patch b/wireshark-0018-sync-pipe-stderr-messages.patch new file mode 100644 index 0000000..fb7dbac --- /dev/null +++ b/wireshark-0018-sync-pipe-stderr-messages.patch @@ -0,0 +1,439 @@ +diff --git a/capchild/capture_sync.c b/capchild/capture_sync.c +index 64c2315..2959f8f 100644 +--- a/capchild/capture_sync.c ++++ b/capchild/capture_sync.c +@@ -106,14 +106,6 @@ static ssize_t pipe_read_block(int pipe_fd, char *indicator, int len, char *msg, + + static void (*fetch_dumpcap_pid)(ws_process_id) = NULL; + +-static void free_argv(char** argv, int argc) +-{ +- int i; +- for (i = 0; i < argc; i++) +- g_free(argv[i]); +- g_free(argv); +-} +- + void + capture_session_init(capture_session *cap_session, capture_file *cf, + new_file_fn new_file, new_packets_fn new_packets, +@@ -222,6 +214,7 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + int sync_pipe[2]; /* pipe used to send messages from child to parent */ + enum PIPES { PIPE_READ, PIPE_WRITE }; /* Constants 0 and 1 for PIPE_READ and PIPE_WRITE */ + #endif ++ char sync_id[ARGV_NUMBER_LEN]; + int sync_pipe_read_fd; + int argc; + char **argv; +@@ -423,14 +416,11 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + } + } + +- /* dumpcap should be running in capture child mode (hidden feature) */ + #ifndef DEBUG_CHILD +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); + #ifdef _WIN32 + g_snprintf(control_id, ARGV_NUMBER_LEN, "%d", GetCurrentProcessId()); + argv = sync_pipe_add_arg(argv, &argc, control_id); + #else +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); + #endif + #endif + +@@ -454,7 +444,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + /* Couldn't create the pipe between parent and child. */ + report_failure("Couldn't create sync pipe: %s", + win32strerror(GetLastError())); +- free_argv(argv, argc); + return FALSE; + } + +@@ -471,7 +460,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + report_failure("Couldn't get C file handle for sync pipe: %s", g_strerror(errno)); + CloseHandle(sync_pipe_read); + CloseHandle(sync_pipe_write); +- free_argv(argv, argc); + return FALSE; + } + +@@ -487,7 +475,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + win32strerror(GetLastError())); + ws_close(sync_pipe_read_fd); /* Should close sync_pipe_read */ + CloseHandle(sync_pipe_write); +- free_argv(argv, argc); + return FALSE; + } + +@@ -505,7 +492,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + ws_close(sync_pipe_read_fd); /* Should close sync_pipe_read */ + CloseHandle(sync_pipe_write); + CloseHandle(signal_pipe); +- free_argv(argv, argc); + return FALSE; + } + +@@ -543,7 +529,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + ws_close(sync_pipe_read_fd); /* Should close sync_pipe_read */ + CloseHandle(sync_pipe_write); + CloseHandle(signal_pipe); +- free_argv(argv, argc); + g_string_free(args, TRUE); + return FALSE; + } +@@ -558,7 +543,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + if (pipe(sync_pipe) < 0) { + /* Couldn't create the pipe between parent and child. */ + report_failure("Couldn't create sync pipe: %s", g_strerror(errno)); +- free_argv(argv, argc); + return FALSE; + } + +@@ -567,8 +551,14 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + * Child process - run dumpcap with the right arguments to make + * it just capture with the specified capture parameters + */ +- dup2(sync_pipe[PIPE_WRITE], 2); ++ //dup2(sync_pipe[PIPE_WRITE], 2); + ws_close(sync_pipe[PIPE_READ]); ++#ifndef DEBUG_CHILD ++ int argc = g_strv_length(argv); ++ argv = sync_pipe_add_arg(argv, &argc, "-Z"); ++ snprintf(sync_id, ARGV_NUMBER_LEN, "%d", sync_pipe[PIPE_WRITE]); ++ argv = sync_pipe_add_arg(argv, &argc, sync_id); ++#endif + execv(argv[0], argv); + if (errno == EPERM || errno == EACCES) + securitymsg = "\nAre you a member of the 'wireshark' group? Try running\n'usermod -a -G wireshark _your_username_' as root."; +@@ -595,7 +585,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + + /* Parent process - read messages from the child process over the + sync pipe. */ +- free_argv(argv, argc); + + /* Close the write side of the pipe, so that only the child has it + open, and thus it completely closes, and thus returns to us +@@ -652,10 +641,11 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf + /* XXX - assumes PIPE_BUF_SIZE > SP_MAX_MSG_LEN */ + #define PIPE_BUF_SIZE 5120 + static int +-sync_pipe_open_command(char* const argv[], int *data_read_fd, ++sync_pipe_open_command(char **argv, int *data_read_fd, + int *message_read_fd, ws_process_id *fork_child, gchar **msg, void(*update_cb)(void)) + { + enum PIPES { PIPE_READ, PIPE_WRITE }; /* Constants 0 and 1 for PIPE_READ and PIPE_WRITE */ ++ char sync_id[ARGV_NUMBER_LEN]; + #ifdef _WIN32 + HANDLE sync_pipe[2]; /* pipe used to send messages from child to parent */ + HANDLE data_pipe[2]; /* pipe used to send data from child to parent */ +@@ -678,6 +668,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + + if (!msg) { + /* We can't return anything */ ++ g_strfreev(argv); + #ifdef _WIN32 + g_string_free(args, TRUE); + #endif +@@ -789,6 +780,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (pipe(sync_pipe) < 0) { + /* Couldn't create the message pipe between parent and child. */ + *msg = g_strdup_printf("Couldn't create sync pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + return -1; + } + +@@ -796,6 +788,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (pipe(data_pipe) < 0) { + /* Couldn't create the data pipe between parent and child. */ + *msg = g_strdup_printf("Couldn't create data pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + ws_close(sync_pipe[PIPE_READ]); + ws_close(sync_pipe[PIPE_WRITE]); + return -1; +@@ -806,18 +799,24 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + * Child process - run dumpcap with the right arguments to make + * it just capture with the specified capture parameters + */ +- dup2(data_pipe[PIPE_WRITE], 1); +- ws_close(data_pipe[PIPE_READ]); +- ws_close(data_pipe[PIPE_WRITE]); +- dup2(sync_pipe[PIPE_WRITE], 2); ++ if (data_read_fd != NULL) { ++ dup2(data_pipe[PIPE_WRITE], 1); ++ ws_close(data_pipe[PIPE_READ]); ++ ws_close(data_pipe[PIPE_WRITE]); ++ } + ws_close(sync_pipe[PIPE_READ]); +- ws_close(sync_pipe[PIPE_WRITE]); ++#ifndef DEBUG_CHILD ++ int argc = g_strv_length(argv); ++ argv = sync_pipe_add_arg(argv, &argc, "-Z"); ++ snprintf(sync_id, ARGV_NUMBER_LEN, "%d", sync_pipe[PIPE_WRITE]); ++ argv = sync_pipe_add_arg(argv, &argc, sync_id); ++#endif + execv(argv[0], argv); + if (errno == EPERM || errno == EACCES) + securitymsg = "\nAre you a member of the 'wireshark' group? Try running\n'usermod -a -G wireshark _your_username_' as root."; + g_snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s%s", + argv[0], g_strerror(errno), securitymsg); +- sync_pipe_errmsg_to_parent(2, errmsg, ""); ++ sync_pipe_errmsg_to_parent(sync_pipe[PIPE_WRITE], errmsg, ""); + + /* Exit with "_exit()", so that we don't close the connection + to the X server (and cause stuff buffered up by our parent but +@@ -829,6 +828,8 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + _exit(1); + } + ++ g_strfreev(argv); ++ + if (fetch_dumpcap_pid && *fork_child > 0) + fetch_dumpcap_pid(*fork_child); + +@@ -904,7 +905,7 @@ sync_pipe_close_command(int *data_read_fd, int *message_read_fd, + /* XXX - assumes PIPE_BUF_SIZE > SP_MAX_MSG_LEN */ + #define PIPE_BUF_SIZE 5120 + static int +-sync_pipe_run_command_actual(char* const argv[], gchar **data, gchar **primary_msg, ++sync_pipe_run_command_actual(char **argv, gchar **data, gchar **primary_msg, + gchar **secondary_msg, void(*update_cb)(void)) + { + gchar *msg; +@@ -1081,7 +1082,7 @@ sync_pipe_run_command_actual(char* const argv[], gchar **data, gchar **primary_m + * redirects to sync_pipe_run_command_actual() + */ + static int +-sync_pipe_run_command(char* const argv[], gchar **data, gchar **primary_msg, ++sync_pipe_run_command(char **argv, gchar **data, gchar **primary_msg, + gchar **secondary_msg, void (*update_cb)(void)) + { + int ret, i; +@@ -1146,22 +1147,14 @@ sync_interface_set_80211_chan(const gchar *iface, const char *freq, const gchar + *primary_msg = g_strdup("Out of mem."); + *secondary_msg = NULL; + *data = NULL; +- free_argv(argv, argc); + return -1; + } + + argv = sync_pipe_add_arg(argv, &argc, "-k"); + argv = sync_pipe_add_arg(argv, &argc, opt); + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif +- + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); + g_free(opt); +- free_argv(argv, argc); + return ret; + } + +@@ -1199,13 +1192,7 @@ sync_interface_list_open(gchar **data, gchar **primary_msg, + /* Ask for the interface list */ + argv = sync_pipe_add_arg(argv, &argc, "-D"); + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); +- free_argv(argv, argc); + return ret; + } + +@@ -1253,13 +1240,7 @@ sync_if_capabilities_open(const gchar *ifname, gboolean monitor_mode, const gcha + argv = sync_pipe_add_arg(argv, &argc, auth); + } + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); +- free_argv(argv, argc); + return ret; + } + +@@ -1298,17 +1279,13 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, gchar ** + argv = sync_pipe_add_arg(argv, &argc, "-S"); + + #ifndef DEBUG_CHILD +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); + #ifdef _WIN32 + create_dummy_signal_pipe(); + argv = sync_pipe_add_arg(argv, &argc, dummy_control_id); +-#else +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); + #endif + #endif + ret = sync_pipe_open_command(argv, data_read_fd, &message_read_fd, + fork_child, msg, update_cb); +- free_argv(argv, argc); + if (ret == -1) { + return -1; + } +diff --git a/dumpcap.c b/dumpcap.c +index 966806e..f177793 100644 +--- a/dumpcap.c ++++ b/dumpcap.c +@@ -123,6 +123,7 @@ static gchar *sig_pipe_name = NULL; + static HANDLE sig_pipe_handle = NULL; + static gboolean signal_pipe_check_running(void); + #endif ++static int sync_pipe_fd = 2; + + #ifdef SIGINFO + static gboolean infodelay; /* if TRUE, don't print capture info in SIGINFO handler */ +@@ -462,7 +463,7 @@ dumpcap_cmdarg_err(const char *fmt, va_list ap) + gchar *msg; + /* Generate a 'special format' message back to parent */ + msg = g_strdup_vprintf(fmt, ap); +- sync_pipe_errmsg_to_parent(2, msg, ""); ++ sync_pipe_errmsg_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else { + fprintf(stderr, "dumpcap: "); +@@ -482,7 +483,7 @@ dumpcap_cmdarg_err_cont(const char *fmt, va_list ap) + if (capture_child) { + gchar *msg; + msg = g_strdup_vprintf(fmt, ap); +- sync_pipe_errmsg_to_parent(2, msg, ""); ++ sync_pipe_errmsg_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else { + vfprintf(stderr, fmt, ap); +@@ -742,7 +743,7 @@ show_filter_code(capture_options *capture_opts) + #endif + if (capture_child) { + /* Let our parent know we succeeded. */ +- pipe_write_block(2, SP_SUCCESS, NULL); ++ pipe_write_block(sync_pipe_fd, SP_SUCCESS, NULL); + } + return TRUE; + } +@@ -783,7 +784,7 @@ print_machine_readable_interfaces(GList *if_list) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- pipe_write_block(2, SP_SUCCESS, NULL); ++ pipe_write_block(sync_pipe_fd, SP_SUCCESS, NULL); + } + + i = 1; /* Interface id number */ +@@ -850,7 +851,7 @@ print_machine_readable_if_capabilities(if_capabilities_t *caps, int queries) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- pipe_write_block(2, SP_SUCCESS, NULL); ++ pipe_write_block(sync_pipe_fd, SP_SUCCESS, NULL); + } + + if (queries & CAPS_QUERY_LINK_TYPES) { +@@ -947,7 +948,7 @@ print_statistics_loop(gboolean machine_readable) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- pipe_write_block(2, SP_SUCCESS, NULL); ++ pipe_write_block(sync_pipe_fd, SP_SUCCESS, NULL); + } + + if (!machine_readable) { +@@ -4655,7 +4656,7 @@ set_80211_channel(const char *iface, const char *opt) + } + + if (capture_child) +- pipe_write_block(2, SP_SUCCESS, NULL); ++ pipe_write_block(sync_pipe_fd, SP_SUCCESS, NULL); + + out: + g_strfreev(options); +@@ -4810,10 +4811,21 @@ main(int argc, char *argv[]) + if (strcmp("-Z", argv[i]) == 0) { + capture_child = TRUE; + machine_readable = TRUE; /* request machine-readable output */ ++ i++; ++ if (i >= argc) { ++ exit_main(1); ++ } ++ ++ if (strcmp(argv[i], SIGNAL_PIPE_CTRL_ID_NONE) != 0) { ++ // get_positive_int calls cmdarg_err ++ if (!ws_strtoi(argv[i], NULL, &sync_pipe_fd) || sync_pipe_fd <= 0) { ++ exit_main(1); ++ } + #ifdef _WIN32 + /* set output pipe to binary mode, to avoid ugly text conversions */ + _setmode(2, O_BINARY); + #endif ++ } + } + } + +@@ -5049,6 +5061,9 @@ main(int argc, char *argv[]) + /*** hidden option: Wireshark child mode (using binary output messages) ***/ + case 'Z': + capture_child = TRUE; ++ /* ++ * Handled above ++ */ + #ifdef _WIN32 + /* set output pipe to binary mode, to avoid ugly text conversions */ + _setmode(2, O_BINARY); +@@ -5491,7 +5506,7 @@ console_log_handler(const char *log_domain, GLogLevelFlags log_level, + /* ERROR, CRITICAL, WARNING, MESSAGE messages goto stderr or */ + /* to parent especially formatted if dumpcap running as child. */ + if (capture_child) { +- sync_pipe_errmsg_to_parent(2, msg, ""); ++ sync_pipe_errmsg_to_parent(sync_pipe_fd, msg, ""); + } else { + fprintf(stderr, "%s", msg); + fflush(stderr); +@@ -5513,7 +5528,7 @@ report_packet_count(unsigned int packet_count) + if (capture_child) { + g_snprintf(count_str, sizeof(count_str), "%u", packet_count); + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Packets: %s", count_str); +- pipe_write_block(2, SP_PACKET_COUNT, count_str); ++ pipe_write_block(sync_pipe_fd, SP_PACKET_COUNT, count_str); + } else { + count += packet_count; + fprintf(stderr, "\rPackets: %u ", count); +@@ -5527,7 +5542,7 @@ report_new_capture_file(const char *filename) + { + if (capture_child) { + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "File: %s", filename); +- pipe_write_block(2, SP_FILE, filename); ++ pipe_write_block(sync_pipe_fd, SP_FILE, filename); + } else { + #ifdef SIGINFO + /* +@@ -5566,7 +5581,7 @@ report_cfilter_error(capture_options *capture_opts, guint i, const char *errmsg) + if (capture_child) { + g_snprintf(tmp, sizeof(tmp), "%u:%s", i, errmsg); + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Capture filter error: %s", errmsg); +- pipe_write_block(2, SP_BAD_FILTER, tmp); ++ pipe_write_block(sync_pipe_fd, SP_BAD_FILTER, tmp); + } else { + /* + * clopts_step_invalid_capfilter in test/suite-clopts.sh MUST match +@@ -5591,7 +5606,7 @@ report_capture_error(const char *error_msg, const char *secondary_error_msg) + "Primary Error: %s", error_msg); + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, + "Secondary Error: %s", secondary_error_msg); +- sync_pipe_errmsg_to_parent(2, error_msg, secondary_error_msg); ++ sync_pipe_errmsg_to_parent(sync_pipe_fd, error_msg, secondary_error_msg); + } else { + cmdarg_err("%s", error_msg); + if (secondary_error_msg[0] != '\0') +@@ -5610,7 +5625,7 @@ report_packet_drops(guint32 received, guint32 pcap_drops, guint32 drops, guint32 + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, + "Packets received/dropped on interface '%s': %u/%u (pcap:%u/dumpcap:%u/flushed:%u/ps_ifdrop:%u)", + name, received, total_drops, pcap_drops, drops, flushed, ps_ifdrop); +- pipe_write_block(2, SP_DROPS, tmp); ++ pipe_write_block(sync_pipe_fd, SP_DROPS, tmp); + g_free(tmp); + } else { + fprintf(stderr, diff --git a/wireshark.spec b/wireshark.spec index 8b80d7b..1becb3e 100644 --- a/wireshark.spec +++ b/wireshark.spec @@ -6,7 +6,7 @@ Summary: Network traffic analyzer Name: wireshark Version: 3.4.10 -Release: 6%{?dist} +Release: 7%{?dist} Epoch: 1 License: GPL+ Url: http://www.wireshark.org/ @@ -36,6 +36,7 @@ Patch0014: wireshark-0014-cve-2023-2858.patch Patch0015: wireshark-0015-cve-2023-2856.patch Patch0016: wireshark-0016-cve-2023-2855.patch Patch0017: wireshark-0017-cve-2023-2952.patch +Patch0018: wireshark-0018-sync-pipe-stderr-messages.patch #install tshark together with wireshark GUI Requires: %{name}-cli = %{epoch}:%{version}-%{release} @@ -283,6 +284,9 @@ fi %{_libdir}/pkgconfig/%{name}.pc %changelog +* Wed Jul 17 2024 Michal Ruprich - 1:3.4.10-7 +- Resolves: RHEL-14596 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe. + * Mon Jun 19 2023 Michal Ruprich - 1:3.4.10-6 - Resolves: #2211413 - XRA dissector infinite loop