dfilter: Fix protocol slices with negative indexes

Field infos have a length property that was not stored with the
field value so when using a negative index the end was computed
from the captured length of the frame tvbuff, leading to incorrect
results. The documentation in wireshark-filter(5) describes how
this was supposed to work but as far as I can tell it never worked
properly.

We now store the length and use that (when it is different from -1)
to locate the end of the protocol data in the tvbuff. An extra wrinkle
is that sometimes the length is set after the field value is created.
This is the most common case as the majority of protocols have a
variable length and dissection generally proceeds with a TVB subset from
the current layer (with offset zero) through all remaining layers to the
end of the captured length. For that reason we must use an expedient to allow
changing the protocol length of an existing protocol fvalue, whenever
proto_item_set_len() is called.

Fixes #17772.

4 files changed, 42 insertions, 19 deletions

diff --git a/epan/ftypes/ftype-protocol.c b/epan/ftypes/ftype-protocol.c
index 0171234905..3b75f1eb52 100644
--- a/epan/ftypes/ftype-protocol.c
+++ b/epan/ftypes/ftype-protocol.c
@@ -23,6 +23,7 @@ value_new(fvalue_t *fv)
 	fv->value.protocol.tvb = NULL;
 	fv->value.protocol.proto_string = NULL;
 	fv->value.protocol.tvb_is_private = FALSE;
+	fv->value.protocol.length = -1;
 }
 
 static void
@@ -31,6 +32,7 @@ value_copy(fvalue_t *dst, const fvalue_t *src)
 	dst->value.protocol.tvb = tvb_clone(src->value.protocol.tvb);
 	dst->value.protocol.proto_string = g_strdup(src->value.protocol.proto_string);
 	dst->value.protocol.tvb_is_private = TRUE;
+	dst->value.protocol.length = src->value.protocol.length;
 }
 
 static void
@@ -43,14 +45,17 @@ value_free(fvalue_t *fv)
 }
 
 static void
-value_set(fvalue_t *fv, tvbuff_t *value, const gchar *name)
+value_set(fvalue_t *fv, tvbuff_t *value, const gchar *name, int length)
 {
-	/* Free up the old value, if we have one */
-	value_free(fv);
+	if (value != NULL) {
+		/* Free up the old value, if we have one */
+		value_free(fv);
 
-	/* Set the protocol description and an (optional, nullable) tvbuff. */
-	fv->value.protocol.tvb = value;
-	fv->value.protocol.proto_string = g_strdup(name);
+		/* Set the protocol description and an (optional, nullable) tvbuff. */
+		fv->value.protocol.tvb = value;
+		fv->value.protocol.proto_string = g_strdup(name);
+	}
+	fv->value.protocol.length = length;
 }
 
 static gboolean
@@ -78,6 +83,7 @@ val_from_string(fvalue_t *fv, const char *s, gchar **err_msg _U_)
 	 * (e.g., proto_expert) */
 	fv->value.protocol.tvb = new_tvb;
 	fv->value.protocol.proto_string = g_strdup("");
+	fv->value.protocol.length = -1;
 	return TRUE;
 }
 
@@ -91,6 +97,7 @@ val_from_literal(fvalue_t *fv, const char *s, gboolean allow_partial_value _U_,
 	value_free(fv);
 	fv->value.protocol.tvb = NULL;
 	fv->value.protocol.proto_string = NULL;
+	fv->value.protocol.length = -1;
 
 	/* Does this look like a byte string? */
 	bytes = byte_array_from_literal(s, err_msg);
@@ -129,6 +136,7 @@ val_from_charconst(fvalue_t *fv, unsigned long num, gchar **err_msg)
 	value_free(fv);
 	fv->value.protocol.tvb = NULL;
 	fv->value.protocol.proto_string = NULL;
+	fv->value.protocol.length = -1;
 
 	/* Does this look like a byte string? */
 	bytes = byte_array_from_charconst(num, err_msg);
@@ -167,7 +175,10 @@ val_to_repr(wmem_allocator_t *scope, const fvalue_t *fv, ftrepr_t rtype _U_, int
 		return NULL;
 
 	TRY {
-		length = tvb_captured_length(fv->value.protocol.tvb);
+		if (fv->value.protocol.length >= 0)
+			length = fv->value.protocol.length;
+		else
+			length = tvb_captured_length(fv->value.protocol.tvb);
 
 		if (length)
 			buf = bytes_to_str_punct_maxlen(scope, tvb_get_ptr(fv->value.protocol.tvb, 0, length), length, ':', 0);
@@ -182,7 +193,9 @@ val_to_repr(wmem_allocator_t *scope, const fvalue_t *fv, ftrepr_t rtype _U_, int
 static gpointer
 value_get(fvalue_t *fv)
 {
-	return fv->value.protocol.tvb;
+	if (fv->value.protocol.length < 0)
+		return fv->value.protocol.tvb;
+	return tvb_new_subset_length_caplen(fv->value.protocol.tvb, 0, fv->value.protocol.length, fv->value.protocol.length);
 }
 
 static guint
@@ -191,8 +204,13 @@ len(fvalue_t *fv)
 	volatile guint length = 0;
 
 	TRY {
-		if (fv->value.protocol.tvb)
-			length = tvb_captured_length(fv->value.protocol.tvb);
+		if (fv->value.protocol.tvb) {
+			if (fv->value.protocol.length >= 0)
+				length = fv->value.protocol.length;
+			else
+				length = tvb_captured_length(fv->value.protocol.tvb);
+
+		}
 	}
 	CATCH_ALL {
 		/* nothing */
@@ -208,6 +226,10 @@ slice(fvalue_t *fv, GByteArray *bytes, guint offset, guint length)
 	const guint8* data;
 
 	if (fv->value.protocol.tvb) {
+		if (fv->value.protocol.length >= 0 && (guint)fv->value.protocol.length < length) {
+			length = (guint)fv->value.protocol.length;
+		}
+
 		TRY {
 			data = tvb_get_ptr(fv->value.protocol.tvb, offset, length);
 			g_byte_array_append(bytes, data, length);
@@ -221,14 +243,14 @@ slice(fvalue_t *fv, GByteArray *bytes, guint offset, guint length)
 }
 
 static int
-_tvbcmp(tvbuff_t *a, tvbuff_t *b)
+_tvbcmp(const protocol_value_t *a, const protocol_value_t *b)
 {
-	guint	a_len = tvb_captured_length(a);
-	guint	b_len = tvb_captured_length(b);
+	guint	a_len = a->length < 0 ? tvb_captured_length(a->tvb) : a->length;
+	guint	b_len = b->length < 0 ? tvb_captured_length(b->tvb) : b->length;
 
 	if (a_len != b_len)
 		return a_len < b_len ? -1 : 1;
-	return memcmp(tvb_get_ptr(a, 0, a_len), tvb_get_ptr(b, 0, a_len), a_len);
+	return memcmp(tvb_get_ptr(a->tvb, 0, a_len), tvb_get_ptr(b->tvb, 0, a_len), a_len);
 }
 
 static int
@@ -240,7 +262,7 @@ cmp_order(const fvalue_t *fv_a, const fvalue_t *fv_b)
 
 	TRY {
 		if ((a->tvb != NULL) && (b->tvb != NULL)) {
-			c = _tvbcmp(a->tvb, b->tvb);
+			c = _tvbcmp(a, b);
 		} else {
 			c = strcmp(a->proto_string, b->proto_string);
 		}
diff --git a/epan/ftypes/ftypes-int.h b/epan/ftypes/ftypes-int.h
index 47654c8127..b397d43682 100644
--- a/epan/ftypes/ftypes-int.h
+++ b/epan/ftypes/ftypes-int.h
@@ -41,7 +41,7 @@ typedef void (*FvalueSetBytesFunc)(fvalue_t*, const guint8 *);
 typedef void (*FvalueSetGuidFunc)(fvalue_t*, const e_guid_t *);
 typedef void (*FvalueSetTimeFunc)(fvalue_t*, const nstime_t *);
 typedef void (*FvalueSetStringFunc)(fvalue_t*, const gchar *value);
-typedef void (*FvalueSetProtocolFunc)(fvalue_t*, tvbuff_t *value, const gchar *name);
+typedef void (*FvalueSetProtocolFunc)(fvalue_t*, tvbuff_t *value, const gchar *name, int length);
 typedef void (*FvalueSetUnsignedIntegerFunc)(fvalue_t*, guint32);
 typedef void (*FvalueSetSignedIntegerFunc)(fvalue_t*, gint32);
 typedef void (*FvalueSetUnsignedInteger64Func)(fvalue_t*, guint64);
diff --git a/epan/ftypes/ftypes.c b/epan/ftypes/ftypes.c
index b651f15dd1..638df51a80 100644
--- a/epan/ftypes/ftypes.c
+++ b/epan/ftypes/ftypes.c
@@ -636,11 +636,11 @@ fvalue_set_string(fvalue_t *fv, const gchar *value)
 }
 
 void
-fvalue_set_protocol(fvalue_t *fv, tvbuff_t *value, const gchar *name)
+fvalue_set_protocol(fvalue_t *fv, tvbuff_t *value, const gchar *name, int length)
 {
 	ws_assert(fv->ftype->ftype == FT_PROTOCOL);
 	ws_assert(fv->ftype->set_value.set_value_protocol);
-	fv->ftype->set_value.set_value_protocol(fv, value, name);
+	fv->ftype->set_value.set_value_protocol(fv, value, name, length);
 }
 
 void
diff --git a/epan/ftypes/ftypes.h b/epan/ftypes/ftypes.h
index c1c1356d7e..c26d58dd93 100644
--- a/epan/ftypes/ftypes.h
+++ b/epan/ftypes/ftypes.h
@@ -232,6 +232,7 @@ ftype_can_is_negative(enum ftenum ftype);
 typedef struct _protocol_value_t
 {
 	tvbuff_t	*tvb;
+	int		length;
 	gchar		*proto_string;
 	gboolean	tvb_is_private;
 } protocol_value_t;
@@ -319,7 +320,7 @@ void
 fvalue_set_string(fvalue_t *fv, const gchar *value);
 
 void
-fvalue_set_protocol(fvalue_t *fv, tvbuff_t *value, const gchar *name);
+fvalue_set_protocol(fvalue_t *fv, tvbuff_t *value, const gchar *name, int length);
 
 void
 fvalue_set_uinteger(fvalue_t *fv, guint32 value);