Do bounds checking of the offset and length in proto_tree_add_string().

Throw an exception if they don't correspond to data available in the
packet - and do so even if the protocol tree argument is null, so that
we catch very long strings that could cause the offset to overflow.

Ask why we try to handle a null pointer passed as the string argument,
while we're at it.

Bug: 14738
Change-Id: I2fa79ad0dcd1f41608844a573e045197ac60aa62
Reviewed-on: https://code.wireshark.org/review/28179
Reviewed-by: Guy Harris <guy@alum.mit.edu>

1 files changed, 16 insertions, 0 deletions

diff --git a/epan/proto.c b/epan/proto.c
index 49960d56e5..67380ab1ed 100644
--- a/epan/proto.c
+++ b/epan/proto.c
@@ -4181,6 +4181,18 @@ proto_tree_add_string(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
 {
 	proto_item	  *pi;
 	header_field_info *hfinfo;
+	gint		  item_length;
+
+	PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo);
+	get_hfi_length(hfinfo, tvb, start, &length, &item_length, ENC_NA);
+	/*
+	 * Special case - if the length is 0, skip the test, so that
+	 * we can have an empty string right after the end of the
+	 * packet.  (This handles URL-encoded forms where the last field
+	 * has no value so the form ends right after the =.)
+	 */
+	if (item_length != 0)
+		test_length(hfinfo, tvb, start, item_length, ENC_NA);
 
 	CHECK_FOR_NULL_TREE(tree);
 
@@ -4245,6 +4257,10 @@ proto_tree_set_string(field_info *fi, const char* value)
 	if (value) {
 		fvalue_set_string(&fi->value, value);
 	} else {
+		/*
+		 * XXX - why is a null value for a string field
+		 * considered valid?
+		 */
 		fvalue_set_string(&fi->value, "[ Null ]");
 	}
 }