LTP : Header and Trailer extension Corrections.

This patchset involves the changes done to decode header and trailer
extensions correctly. There were places where the frame offset was
not handled properly leading to the improper decoding of the subsequent
fields.

Also, the subtree name for Trailer extension was not proper.

Change-Id: Ia38ddddbcd83435ce8aaa73791a2f5a14918b48f
Reviewed-on: https://code.wireshark.org/review/24140
Petri-Dish: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 31 insertions, 33 deletions

diff --git a/epan/dissectors/packet-ltp.c b/epan/dissectors/packet-ltp.c
index 4b9abe48c8..d95a4e4be8 100644
--- a/epan/dissectors/packet-ltp.c
+++ b/epan/dissectors/packet-ltp.c
@@ -1,7 +1,9 @@
 /* packet-ltp.c
  * Routines for LTP dissection
  * Copyright 2009, Mithun Roy <mithunroy13@gmail.com>
- *
+ * Copyright 2017, Krishnamurthy Mayya <krishnamurthymayya@gmail.com>
+     Revision: Minor modifications to Header and Trailer extensions
+               by correcting the offset handling.
  * Wireshark - Network traffic analyzer
  * By Gerald Combs <gerald@wireshark.org>
  * Copyright 1998 Gerald Combs
@@ -270,7 +272,7 @@ dissect_data_segment(proto_tree *ltp_tree, tvbuff_t *tvb,packet_info *pinfo,int
 	tvbuff_t *new_tvb = NULL;
 
 	/* Create a subtree for data segment and add the other fields under it */
-	ltp_data_tree = proto_tree_add_subtree(ltp_tree, tvb, frame_offset, tvb_captured_length(tvb), ett_data_segm, NULL, "Data Segment");
+	ltp_data_tree = proto_tree_add_subtree(ltp_tree, tvb, frame_offset, tvb_captured_length_remaining(tvb, frame_offset), ett_data_segm, NULL, "Data Segment");
 
 
 	/* Client ID - 0 = Bundle Protocol, 1 = CCSDS LTP Service Data Aggregation */
@@ -570,11 +572,9 @@ dissect_cancel_segment(proto_tree * ltp_tree, tvbuff_t *tvb,int frame_offset){
 static int
 dissect_header_extn(proto_tree *ltp_tree, tvbuff_t *tvb,int frame_offset,int hdr_extn_cnt){
 	guint8 extn_type[LTP_MAX_HDR_EXTN];
-	guint64 length[LTP_MAX_HDR_EXTN];
-	guint64 value[LTP_MAX_HDR_EXTN];
+	gint64 length[LTP_MAX_HDR_EXTN];
 
 	int length_size[LTP_MAX_HDR_EXTN];
-	int value_size[LTP_MAX_HDR_EXTN];
 
 	int i;
 	int extn_offset = 0;
@@ -586,30 +586,29 @@ dissect_header_extn(proto_tree *ltp_tree, tvbuff_t *tvb,int frame_offset,int hdr
 		extn_type[i] = tvb_get_guint8(tvb,frame_offset);
 		extn_offset++;
 
-		if((unsigned)(frame_offset + extn_offset) >= tvb_captured_length(tvb)){
+		length[i] = evaluate_sdnv_64(tvb,frame_offset+1,&length_size[i]);
+		if((guint64)(frame_offset + extn_offset + length_size[i] + length[i]) >= (guint64)tvb_captured_length(tvb)){
 			return 0;
 		}
-		length[i] = evaluate_sdnv_64(tvb,frame_offset,&length_size[i]);
+
 		extn_offset += length_size[i];
-		if((unsigned)(frame_offset + extn_offset) >= tvb_captured_length(tvb)){
-			return 0;
-		}
-		value[i] = evaluate_sdnv_64(tvb,frame_offset,&value_size[i]);
-		extn_offset += value_size[i];
-		if((unsigned)(frame_offset + extn_offset) >= tvb_captured_length(tvb)){
-			return 0;
-		}
+		/* From RFC-5326, the total length of the Header Extension Tree will be length of the following:
+			a) Extension type length (1 byte)
+			b) The length of the 'length' field (as defined by the SDNV which handles dynamic size)
+			c) The length of the value field which is the decoded length */
+		extn_offset += (int)length[i];
 	}
 	ltp_hdr_extn_tree = proto_tree_add_subtree(ltp_tree, tvb,frame_offset, extn_offset, ett_hdr_extn, NULL, "Header Extension");
 
 	for(i = 0; i < hdr_extn_cnt; i++){
 		proto_tree_add_uint_format_value(ltp_hdr_extn_tree, hf_ltp_hdr_extn_tag, tvb, frame_offset, 1, extn_type[i], "%x (%s)", extn_type[i], val_to_str_const(extn_type[i],extn_tag_codes,"Unassigned/Reserved"));
+		frame_offset += 1;
 
 		proto_tree_add_uint64_format(ltp_hdr_extn_tree, hf_ltp_hdr_extn_len, tvb, frame_offset, length_size[i],length[i], "Length [%d]: %"G_GINT64_MODIFIER"d",i+1,length[i]);
 		frame_offset += length_size[i];
 
-		proto_tree_add_uint64_format(ltp_hdr_extn_tree, hf_ltp_hdr_extn_val, tvb, frame_offset, value_size[i],value[i], "Value [%d]: %"G_GINT64_MODIFIER"d",i+1,value[i]);
-		frame_offset += value_size[i];
+		proto_tree_add_item (ltp_hdr_extn_tree, hf_ltp_hdr_extn_val, tvb, frame_offset, (int)length[i], ENC_NA);
+		frame_offset += (int)length[i];
 	}
 	return extn_offset;
 }
@@ -617,11 +616,9 @@ dissect_header_extn(proto_tree *ltp_tree, tvbuff_t *tvb,int frame_offset,int hdr
 static int
 dissect_trailer_extn(proto_tree *ltp_tree, tvbuff_t *tvb,int frame_offset,int trl_extn_cnt){
 	guint8 extn_type[LTP_MAX_TRL_EXTN];
-	guint64 length[LTP_MAX_TRL_EXTN];
-	guint64 value[LTP_MAX_TRL_EXTN];
+	gint64 length[LTP_MAX_TRL_EXTN];
 
 	int length_size[LTP_MAX_TRL_EXTN];
-	int value_size[LTP_MAX_TRL_EXTN];
 
 	int i;
 	int extn_offset = 0;
@@ -638,30 +635,31 @@ dissect_trailer_extn(proto_tree *ltp_tree, tvbuff_t *tvb,int frame_offset,int tr
 			return 0;
 		}
 
-		length[i] = evaluate_sdnv_64(tvb,frame_offset,&length_size[i]);
+		length[i] = evaluate_sdnv_64(tvb,frame_offset+1,&length_size[i]);
 		extn_offset += length_size[i];
 
-		if((unsigned)(frame_offset + extn_offset) >= tvb_captured_length(tvb)){
+		if((guint64)(frame_offset + extn_offset + length_size[i] + length[i]) >= tvb_captured_length(tvb)){
 			return 0;
 		}
 
-		value[i] = evaluate_sdnv_64(tvb,frame_offset,&value_size[i]);
-		extn_offset += value_size[i];
-
-		if((unsigned)(frame_offset + extn_offset) >= tvb_captured_length(tvb)){
-			return 0;
-		}
+		/* From RFC-5326, the total length of the Trailer Extension Tree will be length of the following:
+			a) Extension type length (1 byte)
+			b) The length of the 'length' field (as defined by the SDNV which handles dynamic size)
+			c) The length of the value field which is the decoded length */
+		extn_offset += (int)length[i];
 	}
-	ltp_trl_extn_tree = proto_tree_add_subtree(ltp_tree, tvb,frame_offset, extn_offset, ett_trl_extn, NULL, "Header Extension");
+
+	ltp_trl_extn_tree = proto_tree_add_subtree(ltp_tree, tvb,frame_offset, extn_offset, ett_trl_extn, NULL, "Trailer Extension");
 
 	for(i = 0; i < trl_extn_cnt; i++){
 		proto_tree_add_uint_format_value(ltp_trl_extn_tree, hf_ltp_trl_extn_tag, tvb, frame_offset, 1, extn_type[i], "%x (%s)", extn_type[i], val_to_str_const(extn_type[i],extn_tag_codes,"Unassigned/Reserved"));
+		frame_offset += 1;
 
 		proto_tree_add_uint64_format(ltp_trl_extn_tree, hf_ltp_trl_extn_len, tvb, frame_offset, length_size[i], length[i], "Length [%d]: %"G_GINT64_MODIFIER"d",i+1,length[i]);
 		frame_offset += length_size[i];
 
-		proto_tree_add_uint64_format(ltp_trl_extn_tree, hf_ltp_trl_extn_val, tvb, frame_offset, value_size[i], value[i], "Value [%d]: %"G_GINT64_MODIFIER"d",i+0,value[i]);
-		frame_offset += value_size[i];
+		proto_tree_add_item (ltp_trl_extn_tree, hf_ltp_trl_extn_val, tvb, frame_offset, (int)length[i], ENC_NA);
+		frame_offset += (int)length[i];
 	}
 	return extn_offset;
 }
@@ -926,7 +924,7 @@ proto_register_ltp(void)
 	  },
 	  {&hf_ltp_hdr_extn_val,
 		  {"Value","ltp.hdr.extn.val",
-		  FT_UINT64,BASE_DEC,NULL, 0x0, NULL, HFILL}
+		  FT_BYTES,BASE_NONE,NULL, 0x0, NULL, HFILL}
 	  },
 	  {&hf_ltp_trl_extn_tag,
 		  {"Extension tag","ltp.trl.extn.tag",
@@ -938,7 +936,7 @@ proto_register_ltp(void)
 	  },
 	  {&hf_ltp_trl_extn_val,
 		  {"Value","ltp.trl.extn.val",
-		  FT_UINT64,BASE_DEC,NULL, 0x0, NULL, HFILL}
+		  FT_BYTES,BASE_NONE,NULL, 0x0, NULL, HFILL}
 	  },
 	  {&hf_ltp_fragments,
 		  {"LTP Fragments", "ltp.fragments",