Revert the optimization in r52578. As Jakub predicted, it caused at least one

infinite loop to pop up. We're just going to have to eat the performance hit.
(https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9282)

svn path=/trunk/; revision=52660

3 files changed, 44 insertions, 26 deletions

diff --git a/epan/proto.c b/epan/proto.c
index 46039b1425..28d83ed1d5 100644
--- a/epan/proto.c
+++ b/epan/proto.c
@@ -1750,6 +1750,45 @@ ptvcursor_add(ptvcursor_t *ptvc, int hfindex, gint length,
 		offset, length, encoding);
 }
 
+/*
+ * Validates that field length bytes are available starting from
+ * start (pos/neg). Throws an exception if they aren't.
+ */
+static void
+test_length(header_field_info *hfinfo, proto_tree *tree, tvbuff_t *tvb,
+	    gint start, gint length, const guint encoding)
+{
+	gint size = length;
+
+	if (!tvb)
+		return;
+
+	if (hfinfo->type == FT_UINT_BYTES || hfinfo->type == FT_UINT_STRING) {
+		guint32 n;
+
+		n = get_uint_value(tree, tvb, start, length, encoding);
+		if (n > size + n) {
+			/* If n > size + n then we have an integer overflow, so
+			 * set size to -1, which will force the
+			 * tvb_ensure_bytes_exist call below to throw a
+			 * ReportedBoundsError
+			 */
+			size = -1;
+		}
+		else {
+			size += n;
+		}
+	} else if (hfinfo->type == FT_STRINGZ) {
+		/* If we're fetching until the end of the TVB, only validate
+		 * that the offset is within range.
+		 */
+		if (length == -1)
+			size = 0;
+	}
+
+	tvb_ensure_bytes_exist(tvb, start, size);
+}
+
 /* Add an item to a proto_tree, using the text label registered to that item;
    the item is extracted from the tvbuff handed to it. */
 proto_item *
@@ -1761,13 +1800,11 @@ proto_tree_add_item_new(proto_tree *tree, header_field_info *hfinfo, tvbuff_t *t
 
 	DISSECTOR_ASSERT_HINT(hfinfo != NULL, "Not passed hfi!");
 
-	if (tvb) {
-		tvb_ensure_offset_exists(tvb, start);
-	}
+	get_hfi_length(hfinfo, tvb, start, &length, &item_length);
+	test_length(hfinfo, tree, tvb, start, item_length, encoding);
 
 	TRY_TO_FAKE_THIS_ITEM(tree, hfinfo->id, hfinfo);
 
-	get_hfi_length(hfinfo, tvb, start, &length, &item_length);
 	new_fi = new_field_info(tree, hfinfo, tvb, start, item_length);
 
 	if (new_fi == NULL)
@@ -6959,15 +6996,14 @@ proto_tree_add_bits_item(proto_tree *tree, const int hfindex, tvbuff_t *tvb,
 			 const guint encoding)
 {
 	header_field_info *hfinfo;
+	gint		  octet_length;
 	gint		  octet_offset;
 
 	PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo);
 
+	octet_length = (no_of_bits + 7) >> 3;
 	octet_offset = bit_offset >> 3;
-
-	if (tvb) {
-		tvb_ensure_offset_exists(tvb, octet_offset);
-	}
+	test_length(hfinfo, tree, tvb, octet_offset, octet_length, encoding);
 
 	/* Yes, we try to fake this item again in proto_tree_add_bits_ret_val()
 	 * but only after doing a bunch more work (which we can, in the common
diff --git a/epan/tvbuff.c b/epan/tvbuff.c
index b967e81f11..e783746acd 100644
--- a/epan/tvbuff.c
+++ b/epan/tvbuff.c
@@ -510,21 +510,6 @@ tvb_offset_exists(const tvbuff_t *tvb, const gint offset)
 	}
 }
 
-/* Like tvb_offset_exists except it throws an exception instead of returning
- * FALSE */
-void
-tvb_ensure_offset_exists(const tvbuff_t *tvb, const gint offset)
-{
-	guint abs_offset;
-	int exception;
-
-	DISSECTOR_ASSERT(tvb && tvb->initialized);
-
-	exception = compute_offset(tvb, offset, &abs_offset);
-	if (exception)
-		THROW(exception);
-}
-
 guint
 tvb_reported_length(const tvbuff_t *tvb)
 {
diff --git a/epan/tvbuff.h b/epan/tvbuff.h
index d31da15e98..7fef12da6b 100644
--- a/epan/tvbuff.h
+++ b/epan/tvbuff.h
@@ -239,9 +239,6 @@ WS_DLL_PUBLIC void tvb_ensure_bytes_exist(const tvbuff_t *tvb, const gint offset
 /* Checks (w/o throwing exception) that offset exists in buffer */
 WS_DLL_PUBLIC gboolean tvb_offset_exists(const tvbuff_t*, const gint offset);
 
-/* Checks that offset exists in buffer and throws an exception if it doesn't */
-WS_DLL_PUBLIC void tvb_ensure_offset_exists(const tvbuff_t*, const gint offset);
-
 /* Get reported length of buffer */
 WS_DLL_PUBLIC guint tvb_reported_length(const tvbuff_t*);