PROFINET: DCP SET Block with 0 Block Length

If there is DCP SET block with 0 block length, it is dissected
as erroneous block since DCP SET block can not have 0 block
length. Moreover, DCPBlockLength is not decoded if DCP option
and suboption is 0. However, each DCP block must have
Option/Suboption/DCPBlockLength. This is also fixed.

3 files changed, 68 insertions, 66 deletions

diff --git a/plugins/epan/profinet/packet-dcerpc-pn-io.c b/plugins/epan/profinet/packet-dcerpc-pn-io.c
index 95a81ee6fd..7ee1f8fb87 100644
--- a/plugins/epan/profinet/packet-dcerpc-pn-io.c
+++ b/plugins/epan/profinet/packet-dcerpc-pn-io.c
@@ -14500,7 +14500,7 @@ proto_register_pn_io (void)
         NULL, HFILL }
     },
     { &hf_pn_io_mau_type_extension,
-    { "MAU Type Extension", "pn_io.mau_type_extension",
+    { "MAUTypeExtension", "pn_io.mau_type_extension",
         FT_UINT16, BASE_HEX | BASE_RANGE_STRING, RVALS(pn_io_mau_type_extension), 0x0,
         NULL, HFILL }
     },
diff --git a/plugins/epan/profinet/packet-pn-dcp.c b/plugins/epan/profinet/packet-pn-dcp.c
index 73ff74c49a..6a306a938c 100644
--- a/plugins/epan/profinet/packet-pn-dcp.c
+++ b/plugins/epan/profinet/packet-pn-dcp.c
@@ -1090,67 +1090,73 @@ dissect_PNDCP_Suboption_Control(tvbuff_t *tvb, int offset, packet_info *pinfo,
     offset = dissect_pn_uint8(tvb, offset, pinfo, tree, hf_pn_dcp_suboption_control, &suboption);
     offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_length, &block_length);
 
-    switch (suboption) {
-    case PNDCP_SUBOPTION_CONTROL_START_TRANS:
-        pn_append_info(pinfo, dcp_item, ", Start-Trans");
-        proto_item_append_text(block_item, "Control/Start-Transaction");
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
-        break;
-    case PNDCP_SUBOPTION_CONTROL_END_TRANS:
-        pn_append_info(pinfo, dcp_item, ", End-Trans");
-        proto_item_append_text(block_item, "Control/End-Transaction");
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
-        break;
-    case PNDCP_SUBOPTION_CONTROL_SIGNAL:
-        pn_append_info(pinfo, dcp_item, ", Signal");
-        proto_item_append_text(block_item, "Control/Signal");
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
-        block_length -= 2;
+    if (service_id == PNDCP_SERVICE_ID_SET && block_length == 0) {
+        pn_append_info(pinfo, dcp_item, ", Erroneous DCPSet block");
+        proto_item_append_text(block_item, "Control/Erroneous DCPSet block");
+    }
+    else {
+        switch (suboption) {
+        case PNDCP_SUBOPTION_CONTROL_START_TRANS:
+            pn_append_info(pinfo, dcp_item, ", Start-Trans");
+            proto_item_append_text(block_item, "Control/Start-Transaction");
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
+            break;
+        case PNDCP_SUBOPTION_CONTROL_END_TRANS:
+            pn_append_info(pinfo, dcp_item, ", End-Trans");
+            proto_item_append_text(block_item, "Control/End-Transaction");
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
+            break;
+        case PNDCP_SUBOPTION_CONTROL_SIGNAL:
+            pn_append_info(pinfo, dcp_item, ", Signal");
+            proto_item_append_text(block_item, "Control/Signal");
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_block_qualifier, &block_qualifier);
+            block_length -= 2;
 
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_suboption_control_signal_value, &u16SignalValue);
-        break;
-    case PNDCP_SUBOPTION_CONTROL_RESPONSE:
-        proto_item_append_text(block_item, "Control/Response");
-        offset = dissect_PNDCP_Option(tvb, offset, pinfo, tree, block_item, hf_pn_dcp_suboption_control_option,
-            FALSE /* append_col */);
-        block_error = tvb_get_guint8 (tvb, offset);
-        if (tree) {
-            item = proto_tree_add_uint(tree, hf_pn_dcp_block_error, tvb, offset, 1, block_error);
-        }
-        offset += 1;
-        if (block_error != 0) {
-            expert_add_info_format(pinfo, item, &ei_pn_dcp_block_error_unknown, "%s",
-                                    val_to_str(block_error, pn_dcp_block_error, "Unknown"));
-        }
-        info_str = wmem_strdup_printf(wmem_packet_scope(), ", Response(%s)",
-                                      val_to_str(block_error, pn_dcp_block_error, "Unknown"));
-        pn_append_info(pinfo, dcp_item, info_str);
-        proto_item_append_text(block_item, ", BlockError: %s",
-                                    val_to_str(block_error, pn_dcp_block_error, "Unknown"));
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_suboption_control_signal_value, &u16SignalValue);
+            break;
+        case PNDCP_SUBOPTION_CONTROL_RESPONSE:
+            proto_item_append_text(block_item, "Control/Response");
+            offset = dissect_PNDCP_Option(tvb, offset, pinfo, tree, block_item, hf_pn_dcp_suboption_control_option,
+                FALSE /* append_col */);
+            block_error = tvb_get_guint8(tvb, offset);
+            if (tree) {
+                item = proto_tree_add_uint(tree, hf_pn_dcp_block_error, tvb, offset, 1, block_error);
+            }
+            offset += 1;
+            if (block_error != 0) {
+                expert_add_info_format(pinfo, item, &ei_pn_dcp_block_error_unknown, "%s",
+                    val_to_str(block_error, pn_dcp_block_error, "Unknown"));
+            }
+            info_str = wmem_strdup_printf(wmem_packet_scope(), ", Response(%s)",
+                val_to_str(block_error, pn_dcp_block_error, "Unknown"));
+            pn_append_info(pinfo, dcp_item, info_str);
+            proto_item_append_text(block_item, ", BlockError: %s",
+                val_to_str(block_error, pn_dcp_block_error, "Unknown"));
 
-        break;
-    case PNDCP_SUBOPTION_CONTROL_FACT_RESET:
-        pn_append_info(pinfo, dcp_item, ", Reset FactorySettings");
-        proto_item_append_text(block_item, "Control/Reset FactorySettings");
-        block_length -= 2;
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_blockqualifier, &BlockQualifier);
-        proto_item_append_text(block_item, ", BlockQualifier: %s",
-            val_to_str(BlockQualifier, pn_dcp_suboption_other, "reserved"));
-        block_length -= 2;
-        break;
+            break;
+        case PNDCP_SUBOPTION_CONTROL_FACT_RESET:
+            pn_append_info(pinfo, dcp_item, ", Reset FactorySettings");
+            proto_item_append_text(block_item, "Control/Reset FactorySettings");
+            block_length -= 2;
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_blockqualifier, &BlockQualifier);
+            proto_item_append_text(block_item, ", BlockQualifier: %s",
+                val_to_str(BlockQualifier, pn_dcp_suboption_other, "reserved"));
+            block_length -= 2;
+            break;
 
-    case PNDCP_SUBOPTION_CONTROL_RESET_TO_FACT:
-        pn_append_info(pinfo, dcp_item, ", Reset to Factory");
-        proto_item_append_text(block_item, "Reset to FactorySettings");
+        case PNDCP_SUBOPTION_CONTROL_RESET_TO_FACT:
+            pn_append_info(pinfo, dcp_item, ", Reset to Factory");
+            proto_item_append_text(block_item, "Reset to FactorySettings");
 
-        offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_blockqualifier_r2f, &BlockQualifier);
-        proto_item_append_text(block_item, ", BlockQualifier: %s",
-            val_to_str(BlockQualifier, pn_dcp_BlockQualifier, "reserved"));
-        block_length -= 2;
+            offset = dissect_pn_uint16(tvb, offset, pinfo, tree, hf_pn_dcp_blockqualifier_r2f, &BlockQualifier);
+            proto_item_append_text(block_item, ", BlockQualifier: %s",
+                val_to_str(BlockQualifier, pn_dcp_BlockQualifier, "reserved"));
+            block_length -= 2;
 
-        break;
-    default:
-        offset = dissect_pn_undecoded(tvb, offset, pinfo, tree, block_length);
+            break;
+        default:
+            offset = dissect_pn_undecoded(tvb, offset, pinfo, tree, block_length);
+        }
     }
 
     return offset;
@@ -1259,6 +1265,8 @@ dissect_PNDCP_Block(tvbuff_t *tvb, int offset, packet_info *pinfo,
     proto_item *block_item;
     proto_tree *block_tree;
     int         ori_offset = offset;
+    guint8      suboption;
+    guint16     block_length;
 
     /* subtree for block */
     block_item = proto_tree_add_none_format(tree, hf_pn_dcp_block,
@@ -1300,6 +1308,9 @@ dissect_PNDCP_Block(tvbuff_t *tvb, int offset, packet_info *pinfo,
     {
         pn_append_info(pinfo, dcp_item, ", Reserved");
         proto_item_append_text(block_item, "Reserved");
+        offset = dissect_pn_uint8(tvb, offset, pinfo, block_tree, hf_pn_dcp_suboption_control, &suboption);
+        offset = dissect_pn_uint16(tvb, offset, pinfo, block_tree, hf_pn_dcp_block_length, &block_length);
+        offset = dissect_pn_undecoded(tvb, offset, pinfo, block_tree, block_length);
     }
 
     proto_item_set_len(block_item, offset-ori_offset);
diff --git a/plugins/epan/profinet/packet-pn-rsi.c b/plugins/epan/profinet/packet-pn-rsi.c
index 82019b1241..ef43414e71 100644
--- a/plugins/epan/profinet/packet-pn-rsi.c
+++ b/plugins/epan/profinet/packet-pn-rsi.c
@@ -220,15 +220,6 @@ static const range_string pn_rsi_interface[] = {
     { 0, 0, NULL }
 };
 
-/*static const string_string pn_rsi_sw_revision_prefix[] = {
-    { "V", "officially released version" },
-    { "R", "Revision" },
-    { "P", "Prototype" },
-    { "U", "Under Test (Field Test)" },
-    { "T", "Test Device" },
-    { 0, NULL }
-};*/
-
 static int
 dissect_FOpnumOffset(tvbuff_t *tvb, int offset,
     packet_info *pinfo _U_, proto_tree *tree, guint8 *drep _U_, guint32 *u32FOpnumOffset)