From Dirk: fix https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2930 :

Make the image (png, gif, jfif) dissectors "new style" so that they don't
dissect data that does not belong to them.

Modify the HTTP dissector to call heuristic dissectors on the body if the
registered subdissector does not accept/dissect the data.

From me: don't use assert() and don't add a preference to the HTTP dissector
for this behavior: it makes sense to behave like that by default.

svn path=/trunk/; revision=36305

4 files changed, 74 insertions, 123 deletions

diff --git a/epan/dissectors/packet-http.c b/epan/dissectors/packet-http.c
index a2a78882dd..fc1c084564 100644
--- a/epan/dissectors/packet-http.c
+++ b/epan/dissectors/packet-http.c
@@ -610,7 +610,7 @@ dissect_http_message(tvbuff_t *tvb, int offset, packet_info *pinfo,
 	int		datalen;
 	int		reported_datalen = -1;
 	dissector_handle_t handle;
-	gboolean	dissected;
+	gboolean	dissected = FALSE;
 	/*guint		i;*/
 	/*http_info_value_t *si;*/
 	http_eo_t       *eo_info;
@@ -1237,20 +1237,26 @@ dissect_http_message(tvbuff_t *tvb, int offset, packet_info *pinfo,
 				    "multipart/");
 			}
 		}
+
 		if (handle != NULL) {
 			/*
 			 * We have a subdissector - call it.
 			 */
-			dissected = call_dissector(handle, next_tvb, pinfo,
-			    tree);
-		} else {
+			dissected = call_dissector_only(handle, next_tvb, pinfo, tree);
+			if (!dissected)
+				expert_add_info_format(pinfo, http_tree, PI_MALFORMED, PI_NOTE,
+						       "HTTP body subdissector failed, trying heuristic subdissector");
+		}
+
+		if (!dissected) {
 			/*
-			 * We don't have a subdissector - try the heuristic
-			 * subdissectors.
+			 * We don't have a subdissector or we have one and it did not
+			 * dissect the payload - try the heuristic subdissectors.
 			 */
-			dissected = dissector_try_heuristic(
-			    heur_subdissector_list, next_tvb, pinfo, tree);
+			dissected = dissector_try_heuristic(heur_subdissector_list,
+							    next_tvb, pinfo, tree);
 		}
+
 		if (dissected) {
 			/*
 			 * The subdissector dissected the body.
diff --git a/epan/dissectors/packet-image-gif.c b/epan/dissectors/packet-image-gif.c
index ae1c5d3daa..7ee60945b6 100644
--- a/epan/dissectors/packet-image-gif.c
+++ b/epan/dissectors/packet-image-gif.c
@@ -13,21 +13,21 @@
  * Copyright 1998 Gerald Combs
  *
  * Compuserve GIF media decoding functionality provided by Olivier Biot.
- * 
+ *
  * The two GIF specifications are found at several locations, such as W3C:
  * http://www.w3.org/Graphics/GIF/spec-gif87.txt
  * http://www.w3.org/Graphics/GIF/spec-gif89a.txt
- * 
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
  * of the License, or (at your option) any later version.
- * 
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
@@ -80,7 +80,6 @@ static const value_string vals_extensions[] = {
 };
 
 enum {
-	GIF_UNKNOWN = 0,
 	GIF_87a = 0x87,
 	GIF_89a = 0x89,
 };
@@ -130,19 +129,6 @@ static gint ett_extension = -1;
 static gint ett_image = -1;
 
 
-/**************** GIF related declarations and definitions ****************/
-
-
-/************************** Function prototypes **************************/
-
-
-static void
-dissect_gif(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree);
-
-void
-proto_register_gif(void);
-
-
 /****************** GIF protocol dissection functions ******************/
 
 /* There are two Compuserve GIF standards: GIF87a and GIF89a. GIF image files
@@ -150,7 +136,7 @@ proto_register_gif(void);
  * string representation of the version: "GIF87a" or "GIF89a".
  */
 
-static void
+static gint
 dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 {
 	proto_item *ti;
@@ -163,19 +149,24 @@ dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 	guint8 image_bpp;
 	guint tvb_len = tvb_reported_length(tvb);
 	char *str = tvb_get_ephemeral_string(tvb, 0, 6);
-	guint8 version;
+	guint8 version = 0;
+
+	if (tvb_len < 20)
+		return 0;
 
 	/* Check whether we're processing a GIF object */
+	/* see http://www.w3.org/Graphics/GIF/spec-gif89a.txt section 17 */
 	if (strcmp(str, "GIF87a") == 0) {
 		version = GIF_87a;
 	} else if (strcmp(str, "GIF89a") == 0) {
 		version = GIF_89a;
-	} else if (strncmp(str,"GIF", 3) == 0) {
-		version = GIF_UNKNOWN;
 	} else {
 		/* Not a GIF image! */
-		return;
+		return 0;
 	}
+
+	DISSECTOR_ASSERT(version);
+
 	/* Add summary to INFO column if it is enabled */
 	if (check_col(pinfo->cinfo, COL_INFO))
 		col_append_fstr(pinfo->cinfo, COL_INFO, " (%s)", str);
@@ -191,9 +182,6 @@ dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 		gif_tree = proto_item_add_subtree(ti, ett_gif);
 		/* GIF signature */
 		ti = proto_tree_add_item(gif_tree, hf_version, tvb, 0, 6, TRUE);
-		if (version == GIF_UNKNOWN) {
-			proto_item_append_text(ti, " <Error: unknown GIF version>");
-		}
 		/* Screen descriptor */
 		proto_tree_add_item(gif_tree, hf_screen_width, tvb, 6, 2, TRUE);
 		proto_tree_add_item(gif_tree, hf_screen_height, tvb, 8, 2, TRUE);
@@ -209,7 +197,7 @@ dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 		color_map_present = peek & 0x80;
 		color_resolution = 1 + ((peek & 0x60) >> 4);
 		image_bpp = 1 + (peek & 0x07);
-	
+
 		ti = proto_tree_add_text(gif_tree, tvb, 10, 1,
 				"Global settings:");
 		if (color_map_present)
@@ -374,7 +362,7 @@ dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 				color_map_present = peek & 0x80;
 				color_resolution = 1 + ((peek & 0x60) >> 4);
 				image_bpp = 1 + (peek & 0x07);
-	
+
 				ti2 = proto_tree_add_text(subtree, tvb, offset, 1,
 						"Local settings:");
 				if (color_map_present)
@@ -431,23 +419,13 @@ dissect_gif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 			}
 		} /* while */
 	}
+	return offset;
 }
 
 static gboolean
 dissect_gif_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	if (tvb_length(tvb) < 20)
-		return FALSE;
-
-	/* see http://www.w3.org/Graphics/GIF/spec-gif89a.txt section 17 */
-	if ((tvb_strneql(tvb, 0, "GIF89a", 6) == 0) ||
-	    (tvb_strneql(tvb, 0, "GIF87a", 6) == 0))
-	{
-		dissect_gif(tvb, pinfo, tree);
-		return TRUE;
-	}
-
-	return FALSE;
+	return dissect_gif(tvb, pinfo, tree) > 0;
 }
 
 
@@ -701,17 +679,14 @@ proto_register_gif(void)
 	proto_register_field_array(proto_gif, hf, array_length(hf));
 	proto_register_subtree_array(ett, array_length(ett));
 
-	register_dissector(IMG_GIF, dissect_gif, proto_gif);
+	new_register_dissector(IMG_GIF, dissect_gif, proto_gif);
 }
 
 
 void
 proto_reg_handoff_gif(void)
 {
-	dissector_handle_t gif_handle;
-
-	gif_handle = find_dissector(IMG_GIF);
-
+	dissector_handle_t gif_handle = find_dissector(IMG_GIF);
 	/* Register the GIF media type */
 	dissector_add_string("media_type", "image/gif", gif_handle);
 	heur_dissector_add("http", dissect_gif_heur, proto_gif);
diff --git a/epan/dissectors/packet-image-jfif.c b/epan/dissectors/packet-image-jfif.c
index 1d6f69f3bb..14d25cd3cb 100644
--- a/epan/dissectors/packet-image-jfif.c
+++ b/epan/dissectors/packet-image-jfif.c
@@ -13,24 +13,24 @@
  * Copyright 1998 Gerald Combs
  *
  * JFIF media decoding functionality provided by Olivier Biot.
- * 
+ *
  * The JFIF specifications are found at several locations, such as:
  * http://www.jpeg.org/public/jfif.pdf
  * http://www.w3.org/Graphics/JPEG/itu-t81.pdf
  *
  * The Exif specifications are found at several locations, such as:
  * http://www.exif.org/
- * 
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
  * of the License, or (at your option) any later version.
- * 
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
@@ -354,20 +354,6 @@ static gint ett_marker_segment = -1;
 static gint ett_details = -1;
 
 
-/**************** GIF related declarations and definitions ****************/
-
-
-/************************** Function prototypes **************************/
-
-
-static void
-dissect_jfif(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree);
-
-void
-proto_register_jfif(void);
-
-
-
 /****************** JFIF protocol dissection functions ******************/
 
 #define ErrorInvalidJFIF "This is not a valid JFIF (JPEG) object"
@@ -475,7 +461,7 @@ process_sos_header(proto_tree *tree, tvbuff_t *tvb, guint32 len _U_,
 			count--;
 		}
 	}
-	
+
 	proto_tree_add_item(subtree, hf_sos_ss, tvb, offset++, 1, FALSE);
 	proto_tree_add_item(subtree, hf_sos_se, tvb, offset++, 1, FALSE);
 
@@ -771,16 +757,21 @@ process_app2_segment(proto_tree *tree, tvbuff_t *tvb, guint32 len,
 	}
 }
 
-static void
+static gint
 dissect_jfif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 {
 	proto_tree *subtree = NULL;
-	proto_item *ti;
+	proto_item *ti = NULL;
 	guint tvb_len = tvb_reported_length(tvb);
 	guint32 offset = 0;
-	tvbuff_t *tmp_tvb;
-	guint32 len;
-	guint16 marker;
+
+	/* check if we have a full JFIF in tvb */
+	if (tvb_len < 20)
+		return 0;
+	if (tvb_get_ntohs(tvb, 0) != MARKER_SOI)
+		return 0;
+	if (tvb_get_ntohs(tvb, tvb_len-2) != MARKER_EOI)
+		return 0;
 
 	/* Add summary to INFO column if it is enabled */
 	if (check_col(pinfo->cinfo, COL_INFO))
@@ -790,32 +781,23 @@ dissect_jfif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 		ti = proto_tree_add_item(tree, proto_jfif,
 				tvb, 0, -1, FALSE);
 		subtree = proto_item_add_subtree(ti, ett_jfif);
-	}
-
-	marker = tvb_get_ntohs(tvb, 0);
-	if (marker != MARKER_SOI) {
-		if (tree) {
-			proto_tree_add_text(subtree, tvb, 0, 2, ErrorInvalidJFIF);
-			return;
-		}
-	}
-	if (tree)
 		proto_tree_add_item(subtree, hf_marker, tvb, 0, 2, FALSE);
+	}
 
-	offset = 2;
+	offset = 2;		/* skip MARKER_SOI */
 
 	/*
 	 * Process the remaining markers and marker segments
 	 */
 	while (offset < tvb_len) {
 		const char *str;
-		marker = tvb_get_ntohs(tvb, offset);
+		const guint16 marker = tvb_get_ntohs(tvb, offset);
 		str = match_strval(marker, vals_marker);
 		if (str) { /* Known marker */
 			if (marker_has_length(marker)) { /* Marker segment */
 				/* Length of marker segment = 2 + len */
-				len = tvb_get_ntohs(tvb, offset + 2);
-				tmp_tvb = tvb_new_subset(tvb, offset, 2 + len, 2 + len);
+				const guint16 len = tvb_get_ntohs(tvb, offset + 2);
+				tvbuff_t *tmp_tvb = tvb_new_subset(tvb, offset, 2 + len, 2 + len);
 				switch (marker) {
 					case MARKER_APP0:
 						process_app0_segment(subtree, tmp_tvb, len, marker, str);
@@ -849,7 +831,7 @@ dissect_jfif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 						 */
 						proto_tree_add_text(subtree, tvb, offset + 2 + len, -1,
 								"JFIF dissection stops here (dissection of a scan is not yet implemented)");
-						return;
+						return tvb_len;
 						break;
 					default:
 						process_marker_segment(subtree, tmp_tvb, len, marker, str);
@@ -866,25 +848,17 @@ dissect_jfif(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 			ti = proto_tree_add_item(subtree, hf_marker,
 					tvb, offset, 2, FALSE);
 			proto_item_append_text(ti, " (Reserved)");
-			return;
+			return tvb_len;
 		}
 	}
 
-	return;
+	return offset;
 }
 
 static gboolean
 dissect_jfif_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	guint len = tvb_length(tvb);
-	if (len < 20)
-		return FALSE;
-	if (tvb_get_ntohs(tvb, 0) != MARKER_SOI)
-		return FALSE;
-	if (tvb_get_ntohs(tvb, len-2) != MARKER_EOI)
-		return FALSE;
-	dissect_jfif(tvb, pinfo, tree);
-	return TRUE;
+	return dissect_jfif(tvb, pinfo, tree) > 0;
 }
 
 /****************** Register the protocol with Wireshark ******************/
@@ -1078,7 +1052,7 @@ proto_register_jfif(void)
 			{	"Vertical sampling factor",
 				IMG_JFIF ".sof.v_i",
 				FT_UINT8, BASE_DEC, NULL, 0x0F,
-				"Specifies the relationship between the component vertical dimension and maximum image dimension Y.",				
+				"Specifies the relationship between the component vertical dimension and maximum image dimension Y.",
 				HFILL
 			}
 		},
@@ -1156,7 +1130,7 @@ proto_register_jfif(void)
 				HFILL
 			}
 		},
-		{ &hf_sos_al, 
+		{ &hf_sos_al,
 			{	"Successive approximation bit position low or point transform",
 				IMG_JFIF ".sos.al",
 				FT_UINT8, BASE_DEC, NULL, 0x0F,
@@ -1185,16 +1159,14 @@ proto_register_jfif(void)
 	proto_register_field_array(proto_jfif, hf, array_length(hf));
 	proto_register_subtree_array(ett, array_length(ett));
 
-	register_dissector(IMG_JFIF, dissect_jfif, proto_jfif);
+	new_register_dissector(IMG_JFIF, dissect_jfif, proto_jfif);
 }
 
 
 void
 proto_reg_handoff_jfif(void)
 {
-	dissector_handle_t jfif_handle;
-
-	jfif_handle = find_dissector(IMG_JFIF);
+	dissector_handle_t jfif_handle = find_dissector(IMG_JFIF);
 
 	/* Register the JPEG media type */
 	dissector_add_string("media_type", "image/jfif", jfif_handle);
diff --git a/epan/dissectors/packet-image-png.c b/epan/dissectors/packet-image-png.c
index c1882b01f4..243b1611ee 100644
--- a/epan/dissectors/packet-image-png.c
+++ b/epan/dissectors/packet-image-png.c
@@ -218,13 +218,21 @@ static const true_false_string png_chunk_stc = {
 };
 
 
-static void
+static gint
 dissect_png(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree)
 {
 	proto_tree *tree = NULL;
 	proto_item *ti;
 	int offset=0;
 
+	/* http://libpng.org/pub/png/spec/1.2/PNG-Structure.html#PNG-file-signature */
+	static const guint8 magic[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
+	if (tvb_length(tvb) < 20)
+		return 0;
+
+	if (tvb_memeql(tvb, 0, magic, sizeof(magic)) != 0)
+		return 0;
+
 	col_append_str(pinfo->cinfo, COL_INFO, " (PNG)");
 
 	if(parent_tree){
@@ -301,6 +309,7 @@ dissect_png(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree)
 		proto_tree_add_item(chunk_tree, hf_png_chunk_crc, tvb, offset, 4, FALSE);
 		offset+=4;
 	}
+	return offset;
 }
 
 void
@@ -418,24 +427,13 @@ proto_register_png(void)
 
 static gboolean dissect_png_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	/* http://libpng.org/pub/png/spec/1.2/PNG-Structure.html#PNG-file-signature */
-	static const guint8 magic[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
-	if (tvb_length(tvb) < 20)
-		return FALSE;
-
-	if (tvb_memeql(tvb, 0, magic, sizeof(magic)) != 0)
-		return FALSE;
-
-	dissect_png(tvb, pinfo, tree);
-	return TRUE;
+	return dissect_png(tvb, pinfo, tree) > 0;
 }
 
 void
 proto_reg_handoff_png(void)
 {
-	dissector_handle_t png_handle;
-
-	png_handle = create_dissector_handle(dissect_png, proto_png);
+	dissector_handle_t png_handle = new_create_dissector_handle(dissect_png, proto_png);
 	dissector_add_string("media_type", "image/png", png_handle);
 	heur_dissector_add("http", dissect_png_heur, proto_png);
 }