rpms/wireshark
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Resolves: RHEL-14596 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe.

Browse Source
This commit is contained in:
Michal Ruprich 2024-07-17 15:55:24 +02:00
parent a675655675
commit 6ab622d341
3 changed files with 445 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
wireshark-0002-Customize-permission-denied-error.patch
View File

@ -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",

439
wireshark-0018-sync-pipe-stderr-messages.patch Normal file
View File

@ -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,

6
wireshark.spec
View File

@ -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 <mruprich@redhat.com> - 1:3.4.10-7
- Resolves: RHEL-14596 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe.
* Mon Jun 19 2023 Michal Ruprich <mruprich@redhat.com> - 1:3.4.10-6
- Resolves: #2211413 - XRA dissector infinite loop