From a35aa997ad7104c9173c0d6e38d6c00e8eedf19a Mon Sep 17 00:00:00 2001 From: David Perry Date: Tue, 24 Oct 2023 16:51:44 -0400 Subject: [PATCH] RTPS: string dissection, unicode improvements Add a local function `rtps_strlcpy()` which ensures its destination is truncated on a clean unicode character boundary. Use this function for getting the rtps type and topic names, where #19359 was triggered. (Function may also be useful elsewhere in this dissector, but I'm leaving that for other devs in order to reduce gold plating.) Tweak `rtps_util_add_string()` to explicitly dissect the field containing the string's length to make overall dissection more explicit. Also rework `tvb_get_string_enc()`+`proto_tree_add_string()` to `proto_tree_add_item()`. --- epan/dissectors/packet-rtps.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/epan/dissectors/packet-rtps.c b/epan/dissectors/packet-rtps.c index 94e02e6710..5a75a018cb 100644 --- a/epan/dissectors/packet-rtps.c +++ b/epan/dissectors/packet-rtps.c @@ -55,6 +55,7 @@ #include "zlib.h" #include #include +#include #include void proto_register_rtps(void); @@ -915,6 +916,7 @@ static int hf_rtps_parameter_id_toc = -1; static int hf_rtps_parameter_id_rti = -1; static int hf_rtps_parameter_id_adl = -1; static int hf_rtps_parameter_length = -1; +static int hf_rtps_string_length = -1; static int hf_rtps_coherent_set_start = -1; static int hf_rtps_coherent_set_end = -1; static int hf_rtps_param_topic_name = -1; @@ -2803,6 +2805,13 @@ static const fragment_items rtps_frag_items = { static const true_false_string tfs_little_big_endianness = { "Little-Endian", "Big-Endian" }; +/* #19359 - ensure strings we copy aren't truncated halfway through a Unicode codepoint */ +static void rtps_strlcpy(char *dest, const char *src, size_t dest_size) +{ + (void) g_strlcpy(dest, src, dest_size); + ws_utf8_truncate(dest, dest_size); +} + static gint check_offset_addition(gint offset, guint32 value, proto_tree *tree, packet_info *pinfo, tvbuff_t *tvb) { gint new_offset = offset + (gint)value; @@ -4477,15 +4486,15 @@ static void rtps_util_add_generic_guid_v1(proto_tree *tree, tvbuff_t *tvb, gint /* ------------------------------------------------------------------------- */ /* Insert in the protocol tree the next data interpreted as a String * Returns the new offset (after reading the string) + * XXX - should check that string length field makes sense, possibly by + * comparing to a passed-in container length (cf. #19359) */ static gint rtps_util_add_string(proto_tree *tree, tvbuff_t *tvb, gint offset, int hf_item, const guint encoding) { - guint8 *retVal = NULL; - guint32 size = tvb_get_guint32(tvb, offset, encoding); + guint32 size; - retVal = tvb_get_string_enc(wmem_packet_scope(), tvb, offset+4, size, ENC_ASCII); - - proto_tree_add_string(tree, hf_item, tvb, offset, size+4, retVal); + proto_tree_add_item_ret_uint(tree, hf_rtps_string_length, tvb, offset, 4, encoding, &size); + proto_tree_add_item(tree, hf_item, tvb, offset+4, size, ENC_ASCII); /* NDDS align strings at 4-bytes word. So: * string_length: 4 -> buffer_length = 4; @@ -6464,13 +6473,13 @@ static void rtps_util_store_type_mapping(packet_info *pinfo _U_, tvbuff_t *tvb, break; } case TOPIC_INFO_ADD_TOPIC_NAME: { - (void) g_strlcpy(type_mapping_object->topic_name, value, MAX_TOPIC_AND_TYPE_LENGTH); + rtps_strlcpy(type_mapping_object->topic_name, value, MAX_TOPIC_AND_TYPE_LENGTH); type_mapping_object->fields_visited = type_mapping_object->fields_visited | TOPIC_INFO_ADD_TOPIC_NAME; break; } case TOPIC_INFO_ADD_TYPE_NAME: { - (void) g_strlcpy(type_mapping_object->type_name, value, MAX_TOPIC_AND_TYPE_LENGTH); + rtps_strlcpy(type_mapping_object->type_name, value, MAX_TOPIC_AND_TYPE_LENGTH); type_mapping_object->fields_visited = type_mapping_object->fields_visited | TOPIC_INFO_ADD_TYPE_NAME; break; @@ -14776,6 +14785,18 @@ void proto_register_rtps(void) { HFILL } }, + /* String Length ---------------------------------------------------- */ + { &hf_rtps_string_length, { + "String length", + "rtps.param.string.length", + FT_UINT32, + BASE_DEC, + NULL, + 0, + NULL, + HFILL } + }, + /* Parameter / Topic --------------------------------------------------- */ { &hf_rtps_param_topic_name, { "topic",