From 7c9f333ad0f79d695f530e05cdcceceb7d54d545 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sat, 5 May 2018 00:46:20 +0200 Subject: nettrace_3gpp_32_423: fix memleak and copy of uninitialized memory When protocol="map", but the name attribute value is invalid, a memleak occurs. Observe also that dissector_table_str is 22 bytes (21 characters plus nul) and rounding up to a multiple of 4 means that 2 bytes of uninitialized memory could be copied. Avoid that by copying the actual length. Memory leak was found by Clang Static Analyzer. Change-Id: I41f5b104449e108191e505611411a8fb18f1f5db Fixes: v2.1.0rc0-2545-g4b4c7a76c3 ("[Nettrace] Add parsing of some HSS records.") Reviewed-on: https://code.wireshark.org/review/27350 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- wiretap/nettrace_3gpp_32_423.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'wiretap') diff --git a/wiretap/nettrace_3gpp_32_423.c b/wiretap/nettrace_3gpp_32_423.c index 4700bad28a..a00c573f00 100644 --- a/wiretap/nettrace_3gpp_32_423.c +++ b/wiretap/nettrace_3gpp_32_423.c @@ -377,14 +377,12 @@ write_packet_data(wtap_dumper *wdh, wtap_rec *rec, int *err, gchar **err_info, g if (strcmp(proto_name_str, "gtpv2-c") == 0){ /* Change to gtpv2 */ proto_name_str[5] = '\0'; - proto_name_str[6] = '\0'; proto_str_len = 5; } /* XXX Do we need to check for function="S1" */ if (strcmp(proto_name_str, "nas") == 0){ /* Change to nas-eps_plain */ g_strlcpy(proto_name_str, "nas-eps_plain", 14); - proto_name_str[13] = '\0'; proto_str_len = 13; } if (strcmp(proto_name_str, "map") == 0) { @@ -396,7 +394,6 @@ write_packet_data(wtap_dumper *wdh, wtap_rec *rec, int *err, gchar **err_info, g if (strcmp(name_str, "sai_request") == 0) { use_proto_table = TRUE; g_strlcpy(dissector_table_str, "gsm_map.v3.arg.opcode", 22); - dissector_table_str[21] = '\0'; dissector_table_str_len = 21; dissector_table_val = 56; exported_pdu_info->precense_flags = exported_pdu_info->precense_flags + EXP_PDU_TAG_COL_PROT_BIT; @@ -404,10 +401,12 @@ write_packet_data(wtap_dumper *wdh, wtap_rec *rec, int *err, gchar **err_info, g else if (strcmp(name_str, "sai_response") == 0) { use_proto_table = TRUE; g_strlcpy(dissector_table_str, "gsm_map.v3.res.opcode", 22); - dissector_table_str[21] = '\0'; dissector_table_str_len = 21; dissector_table_val = 56; exported_pdu_info->precense_flags = exported_pdu_info->precense_flags + EXP_PDU_TAG_COL_PROT_BIT; + } else { + g_free(exported_pdu_info->proto_col_str); + exported_pdu_info->proto_col_str = NULL; } } /* Find the start of the raw data*/ @@ -465,17 +464,15 @@ write_packet_data(wtap_dumper *wdh, wtap_rec *rec, int *err, gchar **err_info, g packet_buf[1] = 12; /* EXP_PDU_TAG_PROTO_NAME */ packet_buf[2] = 0; packet_buf[3] = tag_str_len; - for (i = 4, j = 0; j < tag_str_len; i++, j++) { - packet_buf[i] = proto_name_str[j]; - } + memcpy(&packet_buf[4], proto_name_str, proto_str_len); + i = 4 + tag_str_len; }else{ packet_buf[0] = 0; packet_buf[1] = 14; /* EXP_PDU_TAG_DISSECTOR_TABLE_NAME */ packet_buf[2] = 0; packet_buf[3] = tag_str_len; - for (i = 4, j = 0; j < tag_str_len; i++, j++) { - packet_buf[i] = dissector_table_str[j]; - } + memcpy(&packet_buf[4], dissector_table_str, dissector_table_str_len); + i = 4 + tag_str_len; packet_buf[i] = 0; i++; packet_buf[i] = EXP_PDU_TAG_DISSECTOR_TABLE_NAME_NUM_VAL; -- cgit v1.2.3