Don't just grab raw string data with tvb_memcpy().

Use proto_tree_add_item...() routines to add strings; use
tvb_get_string_enc() to extract strings.  That way, all strings are
fetched using an encoding value, to properly map to UTF-8.

Change-Id: I2118e812965cfad5d8c288ea40fa50aca9c67fa8
Reviewed-on: https://code.wireshark.org/review/33970
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
This commit is contained in:
Guy Harris 2019-07-16 12:41:32 -07:00
parent 1e630b42e1
commit 6a7b01255a
6 changed files with 29 additions and 45 deletions

View File

@ -403,13 +403,10 @@ tvb_arpproaddr_to_str(tvbuff_t *tvb, gint offset, int ad_len, guint16 type)
return arpproaddr_to_str(tvb_get_ptr(tvb, offset, ad_len), ad_len, type); return arpproaddr_to_str(tvb_get_ptr(tvb, offset, ad_len), ad_len, type);
} }
#define MAX_E164_STR_LEN 20
static const gchar * static const gchar *
atmarpnum_to_str(tvbuff_t *tvb, int offset, int ad_tl) atmarpnum_to_str(tvbuff_t *tvb, int offset, int ad_tl)
{ {
int ad_len = ad_tl & ATMARP_LEN_MASK; int ad_len = ad_tl & ATMARP_LEN_MASK;
gchar *cur;
if (ad_len == 0) if (ad_len == 0)
return "<No address>"; return "<No address>";
@ -418,16 +415,7 @@ atmarpnum_to_str(tvbuff_t *tvb, int offset, int ad_tl)
/* /*
* I'm assuming this means it's an ASCII (IA5) string. * I'm assuming this means it's an ASCII (IA5) string.
*/ */
cur = (gchar *)wmem_alloc(wmem_packet_scope(), MAX_E164_STR_LEN+3+1); return (gchar *) tvb_get_string_enc(wmem_packet_scope(), tvb, offset, ad_len, ENC_ASCII|ENC_NA);
if (ad_len > MAX_E164_STR_LEN) {
/* Can't show it all. */
tvb_memcpy(tvb, cur, offset, MAX_E164_STR_LEN);
g_snprintf(&cur[MAX_E164_STR_LEN], 3+1, "...");
} else {
tvb_memcpy(tvb, cur, offset, ad_len);
cur[ad_len + 1] = '\0';
}
return cur;
} else { } else {
/* /*
* NSAP. * NSAP.

View File

@ -365,7 +365,7 @@ dissect_hsrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_
proto_tree *hsrp_tree; proto_tree *hsrp_tree;
gint offset; gint offset;
guint8 hellotime, holdtime; guint8 hellotime, holdtime;
gchar auth_buf[8 + 1]; gchar *auth;
col_set_str(pinfo->cinfo, COL_PROTOCOL, "HSRP"); col_set_str(pinfo->cinfo, COL_PROTOCOL, "HSRP");
@ -412,12 +412,11 @@ dissect_hsrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_
offset++; offset++;
proto_tree_add_item(hsrp_tree, hf_hsrp_reserved, tvb, offset, 1, ENC_BIG_ENDIAN); proto_tree_add_item(hsrp_tree, hf_hsrp_reserved, tvb, offset, 1, ENC_BIG_ENDIAN);
offset++; offset++;
tvb_memcpy(tvb, auth_buf, offset, 8); auth = (gchar *) tvb_get_string_enc(wmem_packet_scope(), tvb, offset, 8, ENC_ASCII|ENC_NA);
auth_buf[sizeof auth_buf - 1] = '\0'; proto_tree_add_string_format_value(hsrp_tree, hf_hsrp_auth_data, tvb, offset, 8, auth,
proto_tree_add_string_format_value(hsrp_tree, hf_hsrp_auth_data, tvb, offset, 8, auth_buf,
"%sDefault (%s)", "%sDefault (%s)",
(tvb_strneql(tvb, offset, "cisco", strlen("cisco"))) == 0 ? "" : "Non-", (strcmp(auth, "cisco") == 0) ? "" : "Non-",
auth_buf); auth);
offset += 8; offset += 8;
proto_tree_add_item(hsrp_tree, hf_hsrp_virt_ip_addr, tvb, offset, 4, ENC_BIG_ENDIAN); proto_tree_add_item(hsrp_tree, hf_hsrp_virt_ip_addr, tvb, offset, 4, ENC_BIG_ENDIAN);
/* offset += 4; */ /* offset += 4; */
@ -568,7 +567,7 @@ dissect_hsrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_
*/ */
if (tree) { if (tree) {
proto_tree *text_auth_tlv; proto_tree *text_auth_tlv;
gchar auth_buf[8 + 1]; gchar *auth;
ti = proto_tree_add_uint_format_value(hsrp_tree, hf_hsrp2_text_auth_tlv, tvb, offset, 2+len, type, ti = proto_tree_add_uint_format_value(hsrp_tree, hf_hsrp2_text_auth_tlv, tvb, offset, 2+len, type,
"Type=%d Len=%d", type, len); "Type=%d Len=%d", type, len);
@ -577,12 +576,11 @@ dissect_hsrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_
/* Making Text Authentication TLV subtree */ /* Making Text Authentication TLV subtree */
text_auth_tlv = proto_item_add_subtree(ti, ett_hsrp2_text_auth_tlv); text_auth_tlv = proto_item_add_subtree(ti, ett_hsrp2_text_auth_tlv);
tvb_memcpy(tvb, auth_buf, offset, 8); auth = (gchar *) tvb_get_string_enc(wmem_packet_scope(), tvb, offset, 8, ENC_ASCII|ENC_NA);
auth_buf[sizeof auth_buf - 1] = '\0'; proto_tree_add_string_format_value(text_auth_tlv, hf_hsrp2_auth_data, tvb, offset, 8, auth,
proto_tree_add_string_format_value(text_auth_tlv, hf_hsrp2_auth_data, tvb, offset, 8, auth_buf,
"%sDefault (%s)", "%sDefault (%s)",
(tvb_strneql(tvb, offset, "cisco", strlen("cisco"))) == 0 ? "" : "Non-", (strcmp(auth, "cisco") == 0) ? "" : "Non-",
auth_buf); auth);
/* offset += 8; */ /* offset += 8; */
} }
} else if (type == 4 && len == 28) { } else if (type == 4 && len == 28) {

View File

@ -127,8 +127,8 @@ dissect_igap(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d
proto_tree *tree; proto_tree *tree;
proto_item *item; proto_item *item;
guint8 type, tsecs, subtype, asize, msize; guint8 type, tsecs, subtype, asize, msize;
guint8 authentication_result, accounting_status;
int offset = 0; int offset = 0;
guchar account[ACCOUNT_SIZE+1], message[MESSAGE_SIZE+1];
item = proto_tree_add_item(parent_tree, proto_igap, tvb, offset, -1, ENC_NA); item = proto_tree_add_item(parent_tree, proto_igap, tvb, offset, -1, ENC_NA);
tree = proto_item_add_subtree(item, ett_igap); tree = proto_item_add_subtree(item, ett_igap);
@ -177,9 +177,8 @@ dissect_igap(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d
XXX - flag this? */ XXX - flag this? */
asize = ACCOUNT_SIZE; asize = ACCOUNT_SIZE;
} }
tvb_memcpy(tvb, account, offset, asize); /* XXX - encoding? */
account[asize] = '\0'; proto_tree_add_item(tree, hf_account, tvb, offset, asize, ENC_ASCII|ENC_NA);
proto_tree_add_string(tree, hf_account, tvb, offset, asize, account);
} }
offset += ACCOUNT_SIZE; offset += ACCOUNT_SIZE;
@ -189,13 +188,12 @@ dissect_igap(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d
XXX - flag this? */ XXX - flag this? */
msize = MESSAGE_SIZE; msize = MESSAGE_SIZE;
} }
tvb_memcpy(tvb, message, offset, msize);
switch (subtype) { switch (subtype) {
case IGAP_SUBTYPE_PASSWORD_JOIN: case IGAP_SUBTYPE_PASSWORD_JOIN:
case IGAP_SUBTYPE_PASSWORD_LEAVE: case IGAP_SUBTYPE_PASSWORD_LEAVE:
/* Challenge field is user's password */ /* Challenge field is user's password */
message[msize] = '\0'; /* XXX - encoding? */
proto_tree_add_string(tree, hf_igap_user_password, tvb, offset, msize, message); proto_tree_add_item(tree, hf_igap_user_password, tvb, offset, msize, ENC_ASCII|ENC_NA);
break; break;
case IGAP_SUBTYPE_CHALLENGE_RESPONSE_JOIN: case IGAP_SUBTYPE_CHALLENGE_RESPONSE_JOIN:
case IGAP_SUBTYPE_CHALLENGE_RESPONSE_LEAVE: case IGAP_SUBTYPE_CHALLENGE_RESPONSE_LEAVE:
@ -208,11 +206,15 @@ dissect_igap(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* d
break; break;
case IGAP_SUBTYPE_AUTH_MESSAGE: case IGAP_SUBTYPE_AUTH_MESSAGE:
/* Challenge field indicates the result of the authentication */ /* Challenge field indicates the result of the authentication */
proto_tree_add_uint(tree, hf_igap_authentication_result, tvb, offset, msize, message[0]); /* XXX - what if the length isn't 1? */
authentication_result = tvb_get_guint8(tvb, offset);
proto_tree_add_uint(tree, hf_igap_authentication_result, tvb, offset, msize, authentication_result);
break; break;
case IGAP_SUBTYPE_ACCOUNTING_MESSAGE: case IGAP_SUBTYPE_ACCOUNTING_MESSAGE:
/* Challenge field indicates the accounting status */ /* Challenge field indicates the accounting status */
proto_tree_add_uint(tree, hf_igap_accounting_status, tvb, offset, msize, message[0]); /* XXX - what if the length isn't 1? */
accounting_status = tvb_get_guint8(tvb, offset);
proto_tree_add_uint(tree, hf_igap_accounting_status, tvb, offset, msize, accounting_status);
break; break;
default: default:
proto_tree_add_item(tree, hf_igap_unknown_message, tvb, offset, msize, ENC_NA); proto_tree_add_item(tree, hf_igap_unknown_message, tvb, offset, msize, ENC_NA);

View File

@ -713,7 +713,7 @@ dissect_ipdc_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
const char *des; const char *des;
const char *enum_val = ""; const char *enum_val = "";
char tmp_tag_text[IPDC_STR_LEN + 1]; char *tmp_tag_text;
const value_string *val_ptr; const value_string *val_ptr;
gint hf_ptr; gint hf_ptr;
guint32 type; guint32 type;
@ -817,8 +817,7 @@ dissect_ipdc_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
/* simple IPDC_ASCII strings */ /* simple IPDC_ASCII strings */
case IPDC_ASCII: case IPDC_ASCII:
DISSECTOR_ASSERT(len<=IPDC_STR_LEN); DISSECTOR_ASSERT(len<=IPDC_STR_LEN);
tvb_memcpy(tvb, tmp_tag_text, offset+2, len); tmp_tag_text = (char *) tvb_get_string_enc(wmem_packet_scope(), tvb, offset+2, len, ENC_ASCII|ENC_NA);
tmp_tag_text[len] = 0;
proto_tree_add_string_format(tag_tree, hf_ipdc_ascii, tvb, offset, proto_tree_add_string_format(tag_tree, hf_ipdc_ascii, tvb, offset,
len + 2, tmp_tag_text, "%s (0x%2.2x): %s", des, tag, len + 2, tmp_tag_text, "%s (0x%2.2x): %s", des, tag,
tmp_tag_text); tmp_tag_text);

View File

@ -300,17 +300,14 @@ dissect_ipmi_trace(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* da
col_set_str(pinfo->cinfo, COL_INFO, col_set_str(pinfo->cinfo, COL_INFO,
"Channel State Change Notification"); "Channel State Change Notification");
} else if (block_type == HPM2_EMBED_ASCII_MSG) { } else if (block_type == HPM2_EMBED_ASCII_MSG) {
char str[257]; gchar *str;
/* get data length */ /* get data length */
guint str_len = tvb_get_guint8(tvb, 10); guint str_len = tvb_get_guint8(tvb, 10);
if (str_len) { if (str_len) {
/* copy string */ /* copy string */
tvb_memcpy(tvb, str, 11, str_len); str = (gchar *) tvb_get_string_enc(wmem_packet_scope(), tvb, 11, str_len, ENC_ASCII|ENC_NA);
/* pad with nul */
str[str_len] = 0;
/* print the string right inside the column */ /* print the string right inside the column */
col_add_str(pinfo->cinfo, COL_INFO, str); col_add_str(pinfo->cinfo, COL_INFO, str);

View File

@ -953,7 +953,6 @@ zdp_parse_complex_desc(proto_tree *tree, gint ettindex, tvbuff_t *tvb, guint *of
proto_tree *field_tree; proto_tree *field_tree;
gchar *str = (gchar *)wmem_alloc(wmem_packet_scope(), length);
gchar *complex = (gchar *)wmem_alloc(wmem_packet_scope(), max_len); gchar *complex = (gchar *)wmem_alloc(wmem_packet_scope(), max_len);
guint8 tag; guint8 tag;
@ -983,8 +982,9 @@ zdp_parse_complex_desc(proto_tree *tree, gint ettindex, tvbuff_t *tvb, guint *of
g_snprintf(complex, max_len, "<%s>FixMe</%s>", tag_name[tag_icon], tag_name[tag_icon]); g_snprintf(complex, max_len, "<%s>FixMe</%s>", tag_name[tag_icon], tag_name[tag_icon]);
} }
else { else {
tvb_memcpy(tvb, str, *offset+1, length-1); gchar *str;
str[length-1] = '\0';
str = (gchar *) tvb_get_string_enc(wmem_packet_scope(), tvb, *offset+1, length-1, ENC_ASCII|ENC_NA);
/* Handles all string type XML tags. */ /* Handles all string type XML tags. */
if (tag <= tag_icon_url) { if (tag <= tag_icon_url) {
g_snprintf(complex, max_len, "<%s>%s</%s>", tag_name[tag], str, tag_name[tag]); g_snprintf(complex, max_len, "<%s>%s</%s>", tag_name[tag], str, tag_name[tag]);