Fix 2 bugs in heuristic tests which resulted in various false positives;

 I believe this fix addresses the remaining issues for Bug #6047.
  https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6047

Also:
- Fix {-Wshadow] warning;
- Use tvb_get_ntoh24() in 2 places;
- Add ToDo comment about a TCP reassembly issue.

svn path=/trunk/; revision=46338

1 files changed, 17 insertions, 12 deletions

diff --git a/epan/dissectors/packet-reload-framing.c b/epan/dissectors/packet-reload-framing.c
index 8ab70ceb3a..2ca74a9254 100644
--- a/epan/dissectors/packet-reload-framing.c
+++ b/epan/dissectors/packet-reload-framing.c
@@ -97,10 +97,7 @@ get_reload_framing_message_length(packet_info *pinfo _U_, tvbuff_t *tvb, int off
 
 
   if (tvb_get_guint8(tvb, offset) == DATA) {
-
-    length = 1 + 4;
-    length += 3;
-    length += (tvb_get_ntohs(tvb, 1 + 4)<<8)+ tvb_get_guint8(tvb, 1 + 4 + 2);
+    length = 1 + 4 + 3 + tvb_get_ntoh24(tvb, 1 + 4);
   }
 
   return length;
@@ -120,7 +117,7 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
   guint               effective_length;
   guint16             offset;
   conversation_t     *conversation;
-  reload_conv_info_t *reload_framing_info;
+  reload_conv_info_t *reload_framing_info = NULL;
   reload_frame_t *    reload_frame;
   guint8              type;
 
@@ -133,6 +130,8 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
 
   conversation = find_conversation(pinfo->fd->num, &pinfo->src, &pinfo->dst,
                                    pinfo->ptype, pinfo->srcport, pinfo->destport, 0);
+  if (conversation)
+    reload_framing_info = conversation_get_proto_data(conversation, proto_reload_framing);
 
   /* Get the type
    * http://tools.ietf.org/html/draft-ietf-p2psip-base-12
@@ -145,7 +144,10 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
     /* in the data type, check the reload token to be sure this
     *  is a reLoad packet
     */
-    message_length = (tvb_get_ntohs(tvb, 1 + 4)<<8)+ tvb_get_guint8(tvb, 1 + 4 + 2);
+    if (effective_length < 12)  /* [type + seq + length + token] */
+      return 0;
+
+    message_length = tvb_get_ntoh24(tvb, 1 + 4);
     if (message_length < MIN_RELOADDATA_HDR_LENGTH) {
       return 0;
     }
@@ -155,7 +157,8 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
     }
     break;
   case ACK:
-    if (effective_length < 9 || ! conversation) {
+    /* Require previous ACK (i.e., reload_framing_info attached to conversation). */
+    if (effective_length < 9 || ! reload_framing_info) {
       return 0;
     }
     break;
@@ -163,7 +166,6 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
     return 0;
   }
 
-
   /* The message seems to be a valid RELOAD framing message! */
 
   col_set_str(pinfo->cinfo, COL_PROTOCOL, "RELOAD Frame");
@@ -209,13 +211,13 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
   /*
    * Do we already have a state structure for this conv
    */
-  reload_framing_info = conversation_get_proto_data(conversation, proto_reload_framing);
   if (!reload_framing_info) {
     /* No.  Attach that information to the conversation, and add
      * it to the list of information structures.
      */
     reload_framing_info = se_alloc(sizeof(reload_conv_info_t));
-    reload_framing_info->transaction_pdus = se_tree_create_non_persistent(EMEM_TREE_TYPE_RED_BLACK, "reload_framing_transaction_pdus");
+    reload_framing_info->transaction_pdus = se_tree_create_non_persistent(EMEM_TREE_TYPE_RED_BLACK,
+                                                                          "reload_framing_transaction_pdus");
     conversation_add_proto_data(conversation, proto_reload_framing, reload_framing_info);
   }
 
@@ -336,10 +338,8 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr
 
   case ACK:
   {
-    guint32     sequence;
     proto_item *ti_received;
 
-    sequence = tvb_get_ntohl(tvb, offset);
     proto_tree_add_uint(reload_framing_tree, hf_reload_framing_ack_sequence, tvb, offset , 4, sequence);
     offset += 4;
 
@@ -446,6 +446,11 @@ dissect_reload_framing_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                    get_reload_framing_message_length, dissect_reload_framing_message_no_return);
 }
 
+/* ToDo: If a TCP connection is identified heuristically as reload-framing, then
+ *        the code should be such that reload-framing PDUs can be re-assembled (as is
+ *        done for a TCP connection identified as reload-framing because of
+ *        the TCP port used).
+ */
 static gboolean
 dissect_reload_framing_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_)
 {