Ethereal-dev: Re: [Ethereal-dev] Double-free tvb bug in HTTP dissector with gzi pdecompression

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Jerry Talkington <jtalkington@xxxxxxxxxxxxxxxxxxxxx>
Date: Sat, 8 May 2004 05:18:40 -0700
On Sat, May 08, 2004 at 11:21:30AM +0200, Olivier Biot wrote:
> I'll have a closer look once I see the patch ;)

Do'h!

-- 
GPG public key:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x9D5B8762
Index: packet-http.c
===================================================================
RCS file: /cvsroot/ethereal/packet-http.c,v
retrieving revision 1.104
diff -u -r1.104 packet-http.c
--- packet-http.c	7 May 2004 17:36:46 -0000	1.104
+++ packet-http.c	8 May 2004 04:35:06 -0000
@@ -694,7 +694,14 @@
 				/*
 				 * Decompression worked
 				 */
+
+				/* XXX Don't free this, since it's possible
+				 * that the data was only partially
+				 * decompressed, such as when desegmentation
+				 * isn't enabled.
+				 *
 				tvb_free(next_tvb);
+				*/
 				next_tvb = uncomp_tvb;
 				
 				tvb_set_child_real_data_tvbuff(tvb, next_tvb);
@@ -725,6 +732,13 @@
 				
 				goto body_dissected;
 			}
+		} else if (chunks_decoded > 0) {
+			/*
+			 * Add a new data source for the de-chunked data.
+			 */
+			tvb_set_child_real_data_tvbuff(tvb, next_tvb);
+			add_new_data_source(pinfo, next_tvb,
+			    "De-chunked entity body");
 		}
 
 		/*
@@ -894,14 +908,14 @@
 
 		if (linelen <= 0) {
 			/* Can't get the chunk size line */
-			return chunks_decoded;
+			break;
 		}
 
 		chunk_string = tvb_get_string(tvb, offset, linelen);
 
 		if (chunk_string == NULL) {
 			/* Can't get the chunk size line */
-			return chunks_decoded;
+			break;
 		}
 		
 		c = chunk_string;
@@ -915,7 +929,7 @@
 
 		if (sscanf(chunk_string, "%x", &chunk_size) != 1) {
 			g_free(chunk_string);
-			return chunks_decoded;
+			break;
 		}
 
 		g_free(chunk_string);
@@ -1011,7 +1025,13 @@
 		/ * tvb_set_reported_length(new_tvb, chunked_data_size); * /
 		*/
 
+		/* 
+		 * XXX Don't free this, since the tvb that was passed may
+		 * be used if the data spans multiple frames and reassembly
+		 * isn't enabled.
+		 *
 		tvb_free(*tvb_ptr);
+		*/
 		*tvb_ptr = new_tvb;
 		
 	} else {
Index: epan/tvbuff.c
===================================================================
RCS file: /cvsroot/ethereal/epan/tvbuff.c,v
retrieving revision 1.64
diff -u -r1.64 tvbuff.c
--- epan/tvbuff.c	7 May 2004 18:15:24 -0000	1.64
+++ epan/tvbuff.c	8 May 2004 04:35:07 -0000
@@ -2377,10 +2377,14 @@
 		}
 	}
 	
-	if (uncompr != NULL) {
+	if (uncompr != NULL && err == Z_STREAM_END) {
 		uncompr_tvb =  tvb_new_real_data((guint8*) uncompr, bytes_out,
 		    bytes_out);
+		/*
+		 * XXX Setting this here makes the data source disappear.
+		 *
 		tvb_set_free_cb(uncompr_tvb, g_free);
+		*/
 	}
 	g_free(compr);
 	return uncompr_tvb;