From c1cede8d7cb39a80eceb3bc87af0fc4288d50b36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Valverde?= Date: Fri, 21 Oct 2022 13:37:42 +0100 Subject: [PATCH] epan: Format column string input for display. Format the input for display, by escaping some non printable characters, using ws_label_strcpy(). In some cases with vsnprintf() this requires using a temporary buffer. Add some debug checks for invalid UTF-8 errors. The intention here is to pass dissection data directly to the column API, and the column functions are responsible for formatting that data for display. This avoids having to call format_text() before adding a string to a column and separates the concerns better. Display formatting is an UI concern. --- epan/column-utils.c | 54 ++++++++++++++++++++++++++++++--------------- epan/column-utils.h | 33 ++++++++++++++++++++++----- epan/strutil.c | 6 +++++ epan/strutil.h | 3 +++ 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/epan/column-utils.c b/epan/column-utils.c index e80558ded7..b7f3d533d3 100644 --- a/epan/column-utils.c +++ b/epan/column-utils.c @@ -34,11 +34,15 @@ #include #include +#include #ifdef HAVE_LUA #include #endif +#define COL_BUF_MAX_LEN (((COL_MAX_INFO_LEN) > (COL_MAX_LEN)) ? \ + (COL_MAX_INFO_LEN) : (COL_MAX_LEN)) + /* Used for locale decimal point */ static char *col_decimal_point; @@ -411,10 +415,11 @@ col_append_lstr(column_info *cinfo, const gint el, const gchar *str1, ...) va_start(ap, str1); str = str1; do { - if (G_UNLIKELY(str == NULL)) - str = "(null)"; - - pos += g_strlcpy(&col_item->col_buf[pos], str, max_len - pos); + if (G_UNLIKELY(str == NULL)) { + str = "(null)"; + } + WS_UTF_8_CHECK(str, -1); + pos = ws_label_strcpy(col_item->col_buf, max_len, pos, str, 0); } while (pos < max_len && (str = va_arg(ap, const char *)) != COL_ADD_LSTR_TERMINATOR); va_end(ap); @@ -469,6 +474,7 @@ col_do_append_fstr(column_info *cinfo, const int el, const char *separator, cons size_t len, max_len, sep_len; int i; col_item_t* col_item; + char tmp[COL_BUF_MAX_LEN]; sep_len = (separator) ? strlen(separator) : 0; @@ -491,7 +497,7 @@ col_do_append_fstr(column_info *cinfo, const int el, const char *separator, cons * If we have a separator, append it if the column isn't empty. */ if (sep_len != 0 && len != 0) { - (void) g_strlcat(col_item->col_buf, separator, max_len); + (void) ws_label_strcat(col_item->col_buf, max_len, separator, 0); len += sep_len; } @@ -499,8 +505,10 @@ col_do_append_fstr(column_info *cinfo, const int el, const char *separator, cons va_list ap2; va_copy(ap2, ap); - vsnprintf(&col_item->col_buf[len], max_len - len, format, ap2); + vsnprintf(tmp, sizeof(tmp), format, ap2); va_end(ap2); + WS_UTF_8_CHECK(tmp, -1); + ws_label_strcpy(col_item->col_buf, max_len, len, tmp, 0); } } } @@ -541,8 +549,6 @@ col_append_sep_fstr(column_info *cinfo, const gint el, const gchar *separator, } /* Prepends a vararg list to a packet info string. */ -#define COL_BUF_MAX_LEN (((COL_MAX_INFO_LEN) > (COL_MAX_LEN)) ? \ - (COL_MAX_INFO_LEN) : (COL_MAX_LEN)) void col_prepend_fstr(column_info *cinfo, const gint el, const gchar *format, ...) { @@ -552,6 +558,7 @@ col_prepend_fstr(column_info *cinfo, const gint el, const gchar *format, ...) const char *orig; int max_len; col_item_t* col_item; + char tmp[COL_BUF_MAX_LEN]; if (!CHECK_COL(cinfo, el)) return; @@ -572,8 +579,10 @@ col_prepend_fstr(column_info *cinfo, const gint el, const gchar *format, ...) orig = orig_buf; } va_start(ap, format); - vsnprintf(col_item->col_buf, max_len, format, ap); + vsnprintf(tmp, sizeof(tmp), format, ap); va_end(ap); + WS_UTF_8_CHECK(tmp, -1); + ws_label_strcpy(col_item->col_buf, max_len, 0, tmp, 0); /* * Move the fence, unless it's at the beginning of the string. @@ -595,6 +604,7 @@ col_prepend_fence_fstr(column_info *cinfo, const gint el, const gchar *format, . const char *orig; int max_len; col_item_t* col_item; + char tmp[COL_BUF_MAX_LEN]; if (!CHECK_COL(cinfo, el)) return; @@ -615,8 +625,10 @@ col_prepend_fence_fstr(column_info *cinfo, const gint el, const gchar *format, . orig = orig_buf; } va_start(ap, format); - vsnprintf(col_item->col_buf, max_len, format, ap); + vsnprintf(tmp, sizeof(tmp), format, ap); va_end(ap); + WS_UTF_8_CHECK(tmp, -1); + ws_label_strcpy(col_item->col_buf, max_len, 0, tmp, 0); /* * Move the fence if it exists, else create a new fence at the @@ -665,7 +677,8 @@ col_add_str(column_info *cinfo, const gint el, const gchar* str) */ col_item->col_data = col_item->col_buf; } - (void) g_strlcpy(&col_item->col_buf[col_item->col_fence], str, max_len - col_item->col_fence); + WS_UTF_8_CHECK(str, -1); + (void) ws_label_strcpy(col_item->col_buf, max_len, col_item->col_fence, str, 0); } } } @@ -749,10 +762,11 @@ col_add_lstr(column_info *cinfo, const gint el, const gchar *str1, ...) va_start(ap, str1); str = str1; do { - if (G_UNLIKELY(str == NULL)) - str = "(null)"; - - pos += g_strlcpy(&col_item->col_buf[pos], str, max_len - pos); + if (G_UNLIKELY(str == NULL)) { + str = "(null)"; + } + WS_UTF_8_CHECK(str, -1); + pos = ws_label_strcpy(col_item->col_buf, max_len, pos, str, 0); } while (pos < max_len && (str = va_arg(ap, const char *)) != COL_ADD_LSTR_TERMINATOR); va_end(ap); @@ -768,6 +782,7 @@ col_add_fstr(column_info *cinfo, const gint el, const gchar *format, ...) int i; int max_len; col_item_t* col_item; + char tmp[COL_BUF_MAX_LEN]; if (!CHECK_COL(cinfo, el)) return; @@ -793,8 +808,10 @@ col_add_fstr(column_info *cinfo, const gint el, const gchar *format, ...) col_item->col_data = col_item->col_buf; } va_start(ap, format); - vsnprintf(&col_item->col_buf[col_item->col_fence], max_len - col_item->col_fence, format, ap); + vsnprintf(tmp, sizeof(tmp), format, ap); va_end(ap); + WS_UTF_8_CHECK(tmp, -1); + ws_label_strcpy(col_item->col_buf, max_len, col_item->col_fence, tmp, 0); } } } @@ -827,10 +844,11 @@ col_do_append_str(column_info *cinfo, const gint el, const gchar* separator, */ if (separator != NULL) { if (len != 0) { - (void) g_strlcat(col_item->col_buf, separator, max_len); + (void) ws_label_strcat(col_item->col_buf, max_len, separator, 0); } } - (void) g_strlcat(col_item->col_buf, str, max_len); + WS_UTF_8_CHECK(str, -1); + (void) ws_label_strcat(col_item->col_buf, max_len, str, 0); } } } diff --git a/epan/column-utils.h b/epan/column-utils.h index 91c2b9e3dc..33d321e73c 100644 --- a/epan/column-utils.h +++ b/epan/column-utils.h @@ -103,7 +103,7 @@ enum { * @param col the writable column, -1 for checking the state of all columns * @return TRUE if it's writable, FALSE if not */ -WS_DLL_PUBLIC gboolean col_get_writable(column_info *cinfo, const gint col); +WS_DLL_PUBLIC gboolean col_get_writable(column_info *cinfo, const gint col); /** Set the columns writable. * @@ -150,7 +150,10 @@ WS_DLL_PUBLIC const gchar *col_get_text(column_info *cinfo, const gint col); */ WS_DLL_PUBLIC void col_clear(column_info *cinfo, const gint col); -/** Set (replace) the text of a column element, the text won't be copied. +/** Set (replace) the text of a column element, the text won't be formatted or copied. + * + * Use this for simple static strings like protocol names. Don't use for untrusted strings + * or strings that may contain unprintable characters. * * Usually used to set const strings! * @@ -160,7 +163,9 @@ WS_DLL_PUBLIC void col_clear(column_info *cinfo, const gint col); */ WS_DLL_PUBLIC void col_set_str(column_info *cinfo, const gint col, const gchar * str); -/** Add (replace) the text of a column element, the text will be copied. +/** Add (replace) the text of a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. * * @param cinfo the current packet row * @param col the column to use, e.g. COL_INFO @@ -170,9 +175,12 @@ WS_DLL_PUBLIC void col_add_str(column_info *cinfo, const gint col, const gchar * /* terminator argument for col_add_lstr() function */ #define COL_ADD_LSTR_TERMINATOR (const char *) -1 + WS_DLL_PUBLIC void col_add_lstr(column_info *cinfo, const gint el, const gchar *str, ...); /** Add (replace) the text of a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. * * Same function as col_add_str() but using a printf-like format string. * @@ -184,7 +192,9 @@ WS_DLL_PUBLIC void col_add_lstr(column_info *cinfo, const gint el, const gchar * WS_DLL_PUBLIC void col_add_fstr(column_info *cinfo, const gint col, const gchar *format, ...) G_GNUC_PRINTF(3, 4); -/** Append the given text to a column element, the text will be copied. +/** Append the given text to a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. * * @param cinfo the current packet row * @param col the column to use, e.g. COL_INFO @@ -229,6 +239,8 @@ WS_DLL_PUBLIC void col_append_frame_number(packet_info *pinfo, const gint col, c WS_DLL_PUBLIC void col_append_lstr(column_info *cinfo, const gint el, const gchar *str, ...); /** Append the given text to a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. * * Same function as col_append_str() but using a printf-like format string. * @@ -241,6 +253,8 @@ WS_DLL_PUBLIC void col_append_fstr(column_info *cinfo, const gint col, const gch G_GNUC_PRINTF(3, 4); /** Prepend the given text to a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. * * @param cinfo the current packet row * @param col the column to use, e.g. COL_INFO @@ -250,7 +264,10 @@ WS_DLL_PUBLIC void col_append_fstr(column_info *cinfo, const gint col, const gch WS_DLL_PUBLIC void col_prepend_fstr(column_info *cinfo, const gint col, const gchar *format, ...) G_GNUC_PRINTF(3, 4); -/**Prepend the given text to a column element, the text will be formatted and copied. +/** Prepend the given text to a column element, the text will be formatted and copied. + * + * Unprintable characters according to isprint() are escaped. + * * This function is similar to col_prepend_fstr() but this function will * unconditionally set a fence to the end of the prepended data even if there * were no fence before. @@ -262,6 +279,8 @@ WS_DLL_PUBLIC void col_prepend_fence_fstr(column_info *cinfo, const gint col, co G_GNUC_PRINTF(3, 4); /** Append the given text (prepended by a separator) to a column element. + * + * Unprintable characters according to isprint() are escaped. * * Much like col_append_str() but will prepend the given separator if the column isn't empty. * @@ -274,6 +293,8 @@ WS_DLL_PUBLIC void col_append_sep_str(column_info *cinfo, const gint col, const const gchar *str); /** Append the given text (prepended by a separator) to a column element. + * + * Unprintable characters according to isprint() are escaped. * * Much like col_append_fstr() but will prepend the given separator if the column isn't empty. * @@ -297,7 +318,7 @@ WS_DLL_PUBLIC void col_append_sep_fstr(column_info *cinfo, const gint col, const * @param fieldname the fieldname to use for creating a filter (when * applying/preparing/copying as filter) */ -WS_DLL_PUBLIC void col_set_time(column_info *cinfo, const int col, +WS_DLL_PUBLIC void col_set_time(column_info *cinfo, const int col, const nstime_t *ts, const char *fieldname); WS_DLL_PUBLIC void set_fd_time(const struct epan_session *epan, frame_data *fd, gchar *buf); diff --git a/epan/strutil.c b/epan/strutil.c index c0b14bcfb8..5a0d42851b 100644 --- a/epan/strutil.c +++ b/epan/strutil.c @@ -936,6 +936,12 @@ ws_label_strcpy(char *label_str, size_t buf_size, size_t pos, return pos; } +size_t +ws_label_strcat(char *label_str, size_t bufsize, const uint8_t *str, int flags) +{ + return ws_label_strcpy(label_str, bufsize, strlen(label_str), str, flags); +} + /* * Editor modelines - https://www.wireshark.org/tools/modelines.html * diff --git a/epan/strutil.h b/epan/strutil.h index c50762839b..ff46366591 100644 --- a/epan/strutil.h +++ b/epan/strutil.h @@ -195,6 +195,9 @@ void IA5_7BIT_decode(unsigned char * dest, const unsigned char* src, int len); WS_DLL_PUBLIC size_t ws_label_strcpy(char *label_str, size_t bufsize, gsize pos, const uint8_t *str, int flags); +WS_DLL_PUBLIC +size_t ws_label_strcat(char *label_str, size_t bufsize, const uint8_t *str, int flags); + /* * Check name is valid. This covers names for display filter fields, dissector * tables, preference modules, etc. Lower case is preferred.