osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

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

Use proto_tree_add_item_ret_display_string() routines to add strings
if we want to display the string's value in a column, and just use
proto_tree_add_item() if we don't need the string's value.  That way,
all strings are fetched using an encoding value, to properly map to
UTF-8, and, if necessary, are formatted for display.

Add comments asking about encodings.

Change-Id: I32dbdf17c90e77cc080d6132c740c8c5d19ef4c5
Reviewed-on: https://code.wireshark.org/review/33997
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-17 14:48:09 -07:00
parent 2edaca628a
commit fac8c25bb1
1 changed files with 71 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
plugins/epan/profinet/packet-pn-dcp.c
View File

@ -2,6 +2,8 @@
* Routines for PN-DCP (PROFINET Discovery and basic Configuration Protocol)
* packet dissection.
*
* IEC 61158-6-10 section 4.3
*
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1999 Gerald Combs
@ -633,10 +635,20 @@ dissect_PNDCP_Suboption_Device(tvbuff_t *tvb, int offset, packet_info *pinfo,
switch (suboption) {
case PNDCP_SUBOPTION_DEVICE_MANUF:
typeofstation = (char *)wmem_alloc(wmem_packet_scope(), block_length+1);
tvb_memcpy(tvb, (guint8 *) typeofstation, offset, block_length);
typeofstation[block_length] = '\0';
proto_tree_add_string (tree, hf_pn_dcp_suboption_device_typeofstation, tvb, offset, block_length, typeofstation);
/*
* XXX - IEC 61158-6-10 Edition 4.0, section 4.3, says this field
* "shall be coded as data type VisibleString", and that VisibleString
* is "ISO/IEC 646 - International Reference Version without the "del"
* (coding 0x7F) character", i.e. ASCII.
*
* However, at least one capture has a packet where 0xAE is used in
* a place where a registered trademark symbol would be appropriate,
* so the host sending it apparently extended ASCII to ISO 8859-n
* for some value of n. That may have just been an error on their
* part, not realizing that they should have done "(R)" or something
* such as that.
*/
proto_tree_add_item_ret_display_string (tree, hf_pn_dcp_suboption_device_typeofstation, tvb, offset, block_length, ENC_ASCII|ENC_NA, wmem_packet_scope(), &typeofstation);
pn_append_info(pinfo, dcp_item, ", DeviceVendorValue");
proto_item_append_text(block_item, "Device/Manufacturer specific");
if (have_block_qualifier) {
@ -678,10 +690,22 @@ dissect_PNDCP_Suboption_Device(tvbuff_t *tvb, int offset, packet_info *pinfo,
break;
case PNDCP_SUBOPTION_DEVICE_NAMEOFSTATION:
nameofstation = (char *)wmem_alloc(wmem_packet_scope(), block_length+1);
tvb_memcpy(tvb, (guint8 *) nameofstation, offset, block_length);
nameofstation[block_length] = '\0';
proto_tree_add_string (tree, hf_pn_dcp_suboption_device_nameofstation, tvb, offset, block_length, nameofstation);
/*
* XXX - IEC 61158-6-10 Edition 4.0 says, in section 4.3.1.4.15
* "Coding of the field NameOfStationValue", that "This field shall
* be coded as data type OctetString with 1 to 240 octets. The
* definition of IETF RFC 5890 and the following syntax applies: ..."
*
* RFC 5890 means Punycode; should we translate the domain name to
* UTF-8 and show both the untranslated and translated domain name?
*
* They don't mention anything about the RFC 1035 encoding of
* domain names as mentioned in section 3.1 "Name space definitions",
* with the labels being counted strings; does that mean that this
* is just an ASCII string to be interpreted as a Punycode Unicode
* domain name?
*/
proto_tree_add_item_ret_display_string (tree, hf_pn_dcp_suboption_device_nameofstation, tvb, offset, block_length, ENC_ASCII|ENC_NA, wmem_packet_scope(), &nameofstation);
pn_append_info(pinfo, dcp_item, wmem_strdup_printf(wmem_packet_scope(), ", NameOfStation:\"%s\"", nameofstation));
proto_item_append_text(block_item, "Device/NameOfStation");
if (have_block_qualifier) {
@ -803,10 +827,33 @@ dissect_PNDCP_Suboption_Device(tvbuff_t *tvb, int offset, packet_info *pinfo,
}
break;
case PNDCP_SUBOPTION_DEVICE_ALIAS_NAME:
aliasname = (char *)wmem_alloc(wmem_packet_scope(), block_length+1);
tvb_memcpy(tvb, (guint8 *) aliasname, offset, block_length);
aliasname[block_length] = '\0';
proto_tree_add_string (tree, hf_pn_dcp_suboption_device_aliasname, tvb, offset, block_length, aliasname);
/*
* XXX - IEC 61158-6-10 Edition 4.0, section 4.3.1.4.17 "Coding of
* the field AliasNameValue", says this field "shall be coded as
* OctetString. The content shall be the concatenation of the content
* of the fields NameOfPort and NameOfStation.
*
* AliasNameValue = NameOfPort + "." + NameOfStation
*
* " and:
*
* It says in section 4.3.1.4.16 "Coding of the field NameOfPort"
* that "This field shall be coded as OctetString[8] or
* OctetString[14] as "port-xyz" or "port-xyz-rstuv" where x, y,
* z is in the range "0"-"9" from 001 up to 255 and r, s, t, u, v
* is in the range "0"-"9" from 00000 up to 65535. ...
* Furthermore, the definition of IETF RFC 5890 shall be applied."
*
* That suggests that the Octets are probably just ASCII characters;
* IETF RFC 5890 means Punycode, but there isn't anything in those
* string formats that requires non-ASCII characters - they're
* just literally "port-" followed by numbers and hyphens.
*
* It says in section 4.3.1.4.15 "Coding of the field
* NameOfStationValue" that it's a domain name, complete with
* RFC 5890 Punycode.
*/
proto_tree_add_item_ret_display_string (tree, hf_pn_dcp_suboption_device_aliasname, tvb, offset, block_length, ENC_ASCII|ENC_NA, wmem_packet_scope(), &aliasname);
pn_append_info(pinfo, dcp_item, wmem_strdup_printf(wmem_packet_scope(), ", AliasName:\"%s\"", aliasname));
proto_item_append_text(block_item, "Device/AliasName");
if (have_block_qualifier) {
@ -873,7 +920,6 @@ dissect_PNDCP_Suboption_DHCP(tvbuff_t *tvb, int offset, packet_info *pinfo,
guint8 dhcpparameterlength = 0;
guint8 dhcpparameterdata = 0;
guint8 dhcpcontrolparameterdata = 0;
char *arbitraryclientID;
gboolean have_block_info = FALSE;
gboolean have_block_qualifier = FALSE;
@ -922,10 +968,18 @@ dissect_PNDCP_Suboption_DHCP(tvbuff_t *tvb, int offset, packet_info *pinfo,
}
else {
proto_item_append_text(block_item, ", Client-ID: Arbitrary");
arbitraryclientID = (char *)wmem_alloc(wmem_packet_scope(), dhcpparameterlength);
tvb_memcpy(tvb, (guint8 *)arbitraryclientID, offset, dhcpparameterlength - 1);
arbitraryclientID[dhcpparameterlength - 1] = '\0';
proto_tree_add_string(tree, hf_pn_dcp_suboption_dhcp_arbitrary_client_id, tvb, offset, dhcpparameterlength - 1, arbitraryclientID);
/*
* XXX - IEC 61158-6-10 Edition 4.0, section 4.3.1.4.21.5
* "Use of arbitrary client identifier", that this is an
* OctetString to be used as a client identifier with DHCP.
*
* Does that mean it should be FT_BYTES, possibly with
* the BASE_SHOW_ASCII_PRINTABLE flag to show it as ASCII
* iff it's printable? Or should packet-dhcp.c export
* dissect_dhcpopt_client_identifier(), so that we can
* use its heuristics?
*/
proto_tree_add_item(tree, hf_pn_dcp_suboption_dhcp_arbitrary_client_id, tvb, offset, dhcpparameterlength - 1, ENC_ASCII|ENC_NA);
offset += dhcpparameterlength;
}
}