Add some checks for valid option lengths, and for the first option being

a length option.

Break out of the option processing loop if we see an option with the
"end" bit set.

svn path=/trunk/; revision=10353

1 files changed, 128 insertions, 17 deletions

diff --git a/packet-pgm.c b/packet-pgm.c
index 51126dd38a..43662bfe90 100644
--- a/packet-pgm.c
+++ b/packet-pgm.c
@@ -1,7 +1,7 @@
 /* packet-pgm.c
  * Routines for PGM packet disassembly, RFC 3208
  *
- * $Id: packet-pgm.c,v 1.22 2003/12/19 22:46:16 guy Exp $
+ * $Id: packet-pgm.c,v 1.23 2004/03/09 06:46:03 guy Exp $
  *
  * Copyright (c) 2000 by Talarian Corp
  *
@@ -595,11 +595,25 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 	proto_tree *opt_tree = NULL;
 	pgm_opt_length_t opts;
 	pgm_opt_generic_t genopts;
-	int theend = 0, firsttime = 1;
+	gboolean theend = FALSE, firsttime = TRUE;
 
 	tvb_memcpy(tvb, (guint8 *)&opts, offset, sizeof(opts));
+	if (opts.type != PGM_OPT_LENGTH) {
+		proto_tree_add_text(tree, tvb, offset, 1,
+		    "%s Options - initial option is %s, should be %s",
+		    pktname,
+		    val_to_str(opts.type, opt_vals, "Unknown (0x%02x)"),
+		    val_to_str(PGM_OPT_LENGTH, opt_vals, "Unknown (0x%02x)"));
+		return;
+	}
 	opts.total_len = g_ntohs(opts.total_len);
 
+	if (opts.total_len < 4) {
+		proto_tree_add_text(opts_tree, tvb, offset, 4,
+			"%s Options (Total Length %u - invalid, must be >= 4)",
+			pktname, opts.total_len);
+		return;
+	}
 	tf = proto_tree_add_text(tree, tvb, offset,
 		opts.total_len,
 		"%s Options (Total Length %d)", pktname, opts.total_len);
@@ -612,35 +626,60 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		offset+2, 2, opts.total_len);
 
 	offset += 4;
-	for (opts.total_len -= 4; opts.total_len > 0;){
+	for (opts.total_len -= 4; !theend && opts.total_len != 0;){
+		if (opts.total_len < 4) {
+			proto_tree_add_text(opts_tree, tvb, offset, opts.total_len,
+			    "Remaining total options length doesn't have enough for an options header");
+			break;
+		}
 		tvb_memcpy(tvb, (guint8 *)&genopts, offset, sizeof(genopts));
 		if (genopts.type & PGM_OPT_END)  {
 			genopts.type &= ~PGM_OPT_END;
-			theend = 1;
+			theend = TRUE;
+		}
+		if (genopts.len < 4) {
+			proto_tree_add_text(opts_tree, tvb, offset, genopts.len,
+				"Option: %s, Length: %u (invalid, must be >= 4)",
+				val_to_str(genopts.type, opt_vals, "Unknown (0x%02x)"),
+				genopts.len);
+			break;
+		}
+		if (opts.total_len < genopts.len) {
+			proto_tree_add_text(opts_tree, tvb, offset, genopts.len,
+			    "Option: %s, Length: %u (> remaining total options length)",
+			    val_to_str(genopts.type, opt_vals, "Unknown (0x%02x)"),
+			    genopts.len);
+			break;
 		}
 		tf = proto_tree_add_text(opts_tree, tvb, offset, genopts.len,
 			"Option: %s, Length: %u",
 			val_to_str(genopts.type, opt_vals, "Unknown (0x%02x)"),
 			genopts.len);
-		if (genopts.len == 0)
-			break;
 
 		switch(genopts.type) {
 		case PGM_OPT_JOIN:{
 			pgm_opt_join_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_join);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx, tvb,
 				offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_join_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -652,18 +691,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_PARITY_PRM:{
 			pgm_opt_parity_prm_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_parityprm);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint_format(opt_tree, hf_pgm_opt_parity_prm_po, tvb,
 				offset+3, 1, optdata.po, "Parity Parameters: %s (0x%x)",
 				paritystr(optdata.po), optdata.po);
@@ -676,18 +723,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_PARITY_GRP:{
 			pgm_opt_parity_grp_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_paritygrp);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_parity_grp_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -702,18 +757,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 			char nakbuf[8192], *ptr;
 			int i, j, naks, soffset = 0;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_naklist);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type, tvb,
 				offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_nak_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -741,7 +804,7 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 							nakbuf, "List: %s", nakbuf);
 							soffset = 0;
 					}
-					firsttime = 0;
+					firsttime = FALSE;
 				}
 			}
 			if (soffset) {
@@ -762,18 +825,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_PGMCC_DATA:{
 			pgm_opt_pgmcc_data_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_ccdata);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_ccdata_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -811,18 +882,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_PGMCC_FEEDBACK:{
 			pgm_opt_pgmcc_feedback_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_ccdata);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_ccfeedbk_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -860,18 +939,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_NAK_BO_IVL:{
 			pgm_opt_nak_bo_ivl_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_nak_bo_ivl);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx, tvb,
 				offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_nak_bo_ivl_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -886,18 +973,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_NAK_BO_RNG:{
 			pgm_opt_nak_bo_rng_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_nak_bo_rng);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx, tvb,
 				offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_nak_bo_rng_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -912,18 +1007,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_REDIRECT:{
 			pgm_opt_redirect_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_redirect);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx,
 				tvb, offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_redirect_res, tvb,
 				offset+3, 1, optdata.res);
 
@@ -958,18 +1061,26 @@ dissect_pgmopts(tvbuff_t *tvb, int offset, proto_tree *tree,
 		case PGM_OPT_FRAGMENT:{
 			pgm_opt_fragment_t optdata;
 
-			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
 			opt_tree = proto_item_add_subtree(tf, ett_pgm_opts_fragment);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_type,
 				tvb, offset, 1, genopts.type);
 
+			if (genopts.len < sizeof optdata) {
+				proto_tree_add_uint_format(opt_tree, hf_pgm_genopt_len, tvb,
+					offset+1, 1, genopts.len,
+					"Length: %u (bogus, must be >= %u)",
+					genopts.len, sizeof optdata);
+				break;
+			}
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_len, tvb,
 				offset+1, 1, genopts.len);
 
 			proto_tree_add_uint(opt_tree, hf_pgm_genopt_opx, tvb,
 				offset+2, 1, genopts.opx);
 
+			tvb_memcpy(tvb, (guint8 *)&optdata, offset, sizeof(optdata));
+
 			proto_tree_add_uint(opt_tree, hf_pgm_opt_fragment_res, tvb,
 				offset+3, 1, optdata.res);