Define a new exception for reassembly errors, and throw it in several cases

instead of using DISSECTOR_ASSERT. When a dissector passes bad data to the
reassembly machine, that isn't necessarily the dissector's fault - the data may
come straight from the packet, and the dissector may not have enough information
to know it's bad without telling the reassembly machine in the first place.

Also fix a bug in the reassembly machine. If it were given a fragment and all of
the following conditions were met:
- the other associated fragments were already marked as done (reassembled)
- the fragment went beyond the end of the conceptual reassembled buffer
- the dissector had not set the PARTIAL_REASSEMBLY flag
then the reassembly machine would incorrectly think there was an overlap and
run past the end of the already-reassembled buffer.

Should fix the rest of
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8380

#BACKPORT
This is probably too big and intrusive to backport directly, and parts of it
will need adapting anyways since reassemble.c has changed. But the bug exists
and crashes in 1.6 and 1.8, so we'll have to do something.


svn path=/trunk/; revision=48011

3 files changed, 90 insertions, 36 deletions

diff --git a/epan/exceptions.h b/epan/exceptions.h
index 87431a9bf0..938115e622 100644
--- a/epan/exceptions.h
+++ b/epan/exceptions.h
@@ -95,6 +95,15 @@
 **/
 #define OutOfMemoryError	6
 
+/**
+    The reassembly state machine was passed a bad fragment offset,
+    or other similar issues. We used to use DissectorError in these
+    cases, but they're not necessarily the dissector's fault - if the packet
+    contains a bad fragment offset, the dissector shouldn't have to figure
+    that out by itself since that's what the reassembly machine is for.
+**/
+#define ReassemblyError         7
+
 /*
  * Catch errors that, if you're calling a subdissector and catching
  * exceptions from the subdissector, and possibly dissecting more
@@ -114,7 +123,7 @@
  * separately.
  */
 #define CATCH_NONFATAL_ERRORS \
-	CATCH2(ReportedBoundsError, ScsiBoundsError)
+	CATCH3(ReportedBoundsError, ScsiBoundsError, ReassemblyError)
 
 /*
  * Catch all bounds-checking errors.
@@ -128,7 +137,7 @@
  * go all the way to the top level and get reported immediately.
  */
 #define CATCH_BOUNDS_AND_DISSECTOR_ERRORS \
-	CATCH4(BoundsError, ReportedBoundsError, ScsiBoundsError, DissectorError)
+	CATCH5(BoundsError, ReportedBoundsError, ScsiBoundsError, DissectorError, ReassemblyError)
 
 /* Usage:
  *
@@ -287,6 +296,16 @@
 	    (except_state|=EXCEPT_CAUGHT)) \
 		/* user's code goes here */
 
+#define CATCH5(v,w,x,y,z) \
+	if (except_state == 0 && exc != 0 && \
+	    (exc->except_id.except_code == (v) || \
+	     exc->except_id.except_code == (w) || \
+	     exc->except_id.except_code == (x) || \
+	     exc->except_id.except_code == (y) || \
+	     exc->except_id.except_code == (z)) && \
+	    (except_state|=EXCEPT_CAUGHT)) \
+		/* user's code goes here */
+
 #define CATCH_ALL \
 	if (except_state == 0 && exc != 0 && \
 	    (except_state|=EXCEPT_CAUGHT)) \
@@ -307,6 +326,11 @@
 #define THROW_MESSAGE(x, y) \
 	except_throw(XCEPT_GROUP_WIRESHARK, (x), (y))
 
+#define THROW_MESSAGE_ON(cond, x, y) G_STMT_START { \
+	if ((cond)) \
+		except_throw(XCEPT_GROUP_WIRESHARK, (x), (y)); \
+} G_STMT_END
+
 #define GET_MESSAGE			except_message(exc)
 
 #define RETHROW                                     \
diff --git a/epan/reassemble.c b/epan/reassemble.c
index 54e20b10db..3fb07a6d9d 100644
--- a/epan/reassemble.c
+++ b/epan/reassemble.c
@@ -544,7 +544,7 @@ fragment_set_tot_len(const packet_info *pinfo, const guint32 id, GHashTable *fra
 		while (fd) {
 			if (fd->offset > max_offset) {
 				max_offset = fd->offset;
-				DISSECTOR_ASSERT(max_offset <= tot_len);
+				THROW_MESSAGE_ON(max_offset > tot_len, ReassemblyError, "Bad total reassembly block count");
 			}
 			fd = fd->next;
 		}
@@ -553,14 +553,14 @@ fragment_set_tot_len(const packet_info *pinfo, const guint32 id, GHashTable *fra
 		while (fd) {
 			if (fd->offset + fd->len > max_offset) {
 				max_offset = fd->offset + fd->len;
-				DISSECTOR_ASSERT(max_offset <= tot_len);
+				THROW_MESSAGE_ON(max_offset > tot_len, ReassemblyError, "Bad total reassembly length");
 			}
 			fd = fd->next;
 		}
 	}
 
 	if (fd_head->flags & FD_DEFRAGMENTED) {
-		DISSECTOR_ASSERT(max_offset == tot_len);
+		THROW_MESSAGE_ON(max_offset != tot_len, ReassemblyError, "Defragmented complete but total length not satisfied");
 	}
 
 	/* We got this far so the value is sane. */
@@ -715,37 +715,52 @@ fragment_add_work(fragment_data *fd_head, tvbuff_t *tvb, const int offset,
 	fragment_data *fd_i;
 	guint32 max, dfpos;
 	unsigned char *old_data;
+	const char *error = NULL;
 
 	/* create new fd describing this fragment */
 	fd = g_slice_new(fragment_data);
 	fd->next = NULL;
 	fd->flags = 0;
 	fd->frame = pinfo->fd->num;
-	if (fd->frame > fd_head->frame)
-		fd_head->frame = fd->frame;
 	fd->offset = frag_offset;
 	fd->len  = frag_data_len;
 	fd->data = NULL;
 
-	/*
-	 * If it was already defragmented and this new fragment goes beyond
-	 * data limits, set flag in already empty fds & point old fds to malloc'ed data.
-	 */
-	if(fd_head->flags & FD_DEFRAGMENTED && (frag_offset+frag_data_len) >= fd_head->datalen &&
-		fd_head->flags & FD_PARTIAL_REASSEMBLY){
-		for(fd_i=fd_head->next; fd_i; fd_i=fd_i->next){
-			if( !fd_i->data ) {
-				fd_i->data = fd_head->data + fd_i->offset;
-				fd_i->flags |= FD_NOT_MALLOCED;
+	/* If it was already defragmented and this new fragment goes beyond the
+	 * old data limits... */
+	if(fd_head->flags & FD_DEFRAGMENTED && (frag_offset+frag_data_len) >= fd_head->datalen) {
+		/* If we've been requested to continue reassembly, set flag in
+		 * already empty fds & point old fds to malloc'ed data. */
+		if (fd_head->flags & FD_PARTIAL_REASSEMBLY) {
+			for(fd_i=fd_head->next; fd_i; fd_i=fd_i->next){
+				if( !fd_i->data ) {
+					fd_i->data = fd_head->data + fd_i->offset;
+					fd_i->flags |= FD_NOT_MALLOCED;
+				}
+				fd_i->flags &= (~FD_TOOLONGFRAGMENT) & (~FD_MULTIPLETAILS);
 			}
-			fd_i->flags &= (~FD_TOOLONGFRAGMENT) & (~FD_MULTIPLETAILS);
+			fd_head->flags &= ~(FD_DEFRAGMENTED|FD_PARTIAL_REASSEMBLY|FD_DATALEN_SET);
+			fd_head->flags &= (~FD_TOOLONGFRAGMENT) & (~FD_MULTIPLETAILS);
+			fd_head->datalen=0;
+			fd_head->reassembled_in=0;
+		}
+		else {
+			/* Otherwise, bail out since we have no idea what to do
+			 * with this fragment (and if we keep going we'll run
+			 * past the end of a buffer sooner or later).
+			 *
+			 * XXX: Is ReportedBoundsError the right thing to throw?
+			 */
+			g_slice_free(fragment_data, fd);
+			THROW(ReportedBoundsError);
 		}
-		fd_head->flags &= ~(FD_DEFRAGMENTED|FD_PARTIAL_REASSEMBLY|FD_DATALEN_SET);
-		fd_head->flags &= (~FD_TOOLONGFRAGMENT) & (~FD_MULTIPLETAILS);
-		fd_head->datalen=0;
-		fd_head->reassembled_in=0;
 	}
 
+	/* Do this after we may have bailed out (above) so that we don't leave
+	 * fd_head->frame in a bad state if we do */
+	if (fd->frame > fd_head->frame)
+		fd_head->frame = fd->frame;
+
 	if (!more_frags) {
 		/*
 		 * This is the tail fragment in the sequence.
@@ -878,19 +893,13 @@ fragment_add_work(fragment_data *fd_head, tvbuff_t *tvb, const int offset,
 
 			if ( fd_i->offset+fd_i->len > dfpos ) {
 				if (fd_i->offset+fd_i->len > max)
-					g_warning("Reassemble error in frame %u: offset %u + len %u > max %u",
-						pinfo->fd->num, fd_i->offset,
-						fd_i->len, max);
+					error = "offset + len > max";
 				else if (dfpos < fd_i->offset)
-					g_warning("Reassemble error in frame %u: dfpos %u < offset %u",
-						pinfo->fd->num, dfpos, fd_i->offset);
+					error = "dfpos < offset";
 				else if (dfpos-fd_i->offset > fd_i->len)
-					g_warning("Reassemble error in frame %u: dfpos %u - offset %u > len %u",
-						pinfo->fd->num, dfpos, fd_i->offset,
-						fd_i->len);
+					error = "dfpos - offset > len";
 				else if (!fd_head->data)
-					g_warning("Reassemble error in frame %u: no data",
-						pinfo->fd->num);
+					error = "no data";
 				else {
 					if (fd_i->offset < dfpos) {
 						fd_i->flags    |= FD_OVERLAP;
@@ -908,10 +917,9 @@ fragment_add_work(fragment_data *fd_head, tvbuff_t *tvb, const int offset,
 						fd_i->len-(dfpos-fd_i->offset));
 				}
 			} else {
-				if (fd_i->offset + fd_i->len < fd_i->offset) /* Integer overflow? */
-					g_warning("Reassemble error in frame %u: offset %u + len %u < offset",
-						pinfo->fd->num, fd_i->offset,
-						fd_i->len);
+				if (fd_i->offset + fd_i->len < fd_i->offset) { /* Integer overflow? */
+					error = "offset + len < offset";
+				}
 			}
 			if( fd_i->flags & FD_NOT_MALLOCED )
 				fd_i->flags &= ~FD_NOT_MALLOCED;
@@ -929,6 +937,11 @@ fragment_add_work(fragment_data *fd_head, tvbuff_t *tvb, const int offset,
 	fd_head->flags |= FD_DEFRAGMENTED;
 	fd_head->reassembled_in=pinfo->fd->num;
 
+	/* we don't throw until here to avoid leaking old_data and others */
+        if (error) {
+		THROW_MESSAGE(ReassemblyError, error);
+	}
+
 	return TRUE;
 }
 
diff --git a/epan/show_exception.c b/epan/show_exception.c
index 68e061eb67..ddc41bff85 100644
--- a/epan/show_exception.c
+++ b/epan/show_exception.c
@@ -109,6 +109,23 @@ show_exception(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
 		        dissector_error_nomsg : exception_message);
 		break;
 
+	case ReassemblyError:
+		col_append_fstr(pinfo->cinfo, COL_INFO,
+		    "[Reassembly error, protocol %s: %s]",
+		    pinfo->current_proto,
+		    exception_message == NULL ?
+		        dissector_error_nomsg : exception_message);
+		item = proto_tree_add_protocol_format(tree, proto_malformed, tvb, 0, 0,
+		    "[Reassembly error, protocol %s: %s]",
+		    pinfo->current_proto,
+		    exception_message == NULL ?
+		        dissector_error_nomsg : exception_message);
+		expert_add_info_format(pinfo, item, PI_MALFORMED, PI_ERROR,
+		    "%s",
+		    exception_message == NULL ?
+		        dissector_error_nomsg : exception_message);
+		break;
+
 	default:
 		/* XXX - we want to know, if an unknown exception passed until here, don't we? */
 		g_assert_not_reached();