HTTP: fix bad reassembly with Content-Type and no Content-Length

Any request or response with the Content-Type header and no
Content-Length header would cause the HTTP dissector to combine all
segments until the end of the connection. This is bogus, it should only
do this for HTTP responses under stricter conditions.

To fix this issue: 1) explicitly disable body desegmentation for
messages that never have a message body, 2) restrict "desegmentat until
the end" to HTTP responses.

The "Connection: Keep-Alive" case was a fix for bug 1142, but that is
now properly addressed by checking for the 304 status code.

Bug: 13116
Change-Id: I02371ac88ec2de6ee966fdc6df0dd246ad49c46d
Reviewed-on: https://code.wireshark.org/review/33035
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

5 files changed, 85 insertions, 42 deletions

diff --git a/epan/dissectors/packet-http.c b/epan/dissectors/packet-http.c
index f5ad5e6ae9..f48e898411 100644
--- a/epan/dissectors/packet-http.c
+++ b/epan/dissectors/packet-http.c
@@ -301,6 +301,7 @@ typedef struct {
 	char    *upgrade;
 } headers_t;
 
+static gint parse_http_status_code(const guchar *line, const guchar *lineend);
 static int is_http_request_or_reply(const gchar *data, int linelen,
 				    http_type_t *type, ReqRespDissector
 				    *reqresp_dissector, http_conv_t *conv_data);
@@ -1126,7 +1127,7 @@ dissect_http_message(tvbuff_t *tvb, int offset, packet_info *pinfo,
 		 * desegmentation if we're told to.
 		 */
 		if (!req_resp_hdrs_do_reassembly(tvb, offset, pinfo,
-		    http_desegment_headers, http_desegment_body)) {
+		    http_desegment_headers, http_desegment_body, FALSE)) {
 			/*
 			 * More data needed for desegmentation.
 			 */
@@ -1158,15 +1159,35 @@ dissect_http_message(tvbuff_t *tvb, int offset, packet_info *pinfo,
 		/*
 		 * Do header desegmentation if we've been told to,
 		 * and do body desegmentation if we've been told to and
-		 * we find a Content-Length header. Responses to HEAD MUST NOT
-		 * contain a message body, so ignore the Content-Length header
-		 * which is done by disabling body desegmentation.
+		 * we find a Content-Length header in requests.
+		 *
+		 * The following cases (from RFC 7230, Section 3.3) never have a
+		 * response body, so do not attempt to desegment the body for:
+		 * * Responses to HEAD requests.
+		 * * 2xx responses to CONNECT requests.
+		 * * 1xx, 204 No Content, 304 Not Modified responses.
+		 *
+		 * Additionally if we are at the end of stream, no more segments
+		 * will be added so disable body segmentation too in that case.
 		 */
-		try_desegment_body = (http_desegment_body &&
-			!(http_type == HTTP_RESPONSE && conv_data->request_method && g_str_equal(conv_data->request_method, "HEAD")) &&
-			!end_of_stream);
+		try_desegment_body = (http_desegment_body && !end_of_stream);
+		if (try_desegment_body && http_type == HTTP_RESPONSE) {
+			/*
+			 * conv_data->response_code is not yet set, so extract
+			 * the response code from the current line.
+			 */
+			gint response_code = parse_http_status_code(firstline, firstline + first_linelen);
+			if ((g_strcmp0(conv_data->request_method, "HEAD") == 0 ||
+				(response_code / 100 == 2 && g_strcmp0(conv_data->request_method, "CONNECT") == 0) ||
+				response_code / 100 == 1 ||
+				response_code == 204 ||
+				response_code == 304)) {
+				/* No response body is present. */
+				try_desegment_body = FALSE;
+			}
+		}
 		if (!req_resp_hdrs_do_reassembly(tvb, offset, pinfo,
-		    http_desegment_headers, try_desegment_body)) {
+		    http_desegment_headers, try_desegment_body, http_type == HTTP_RESPONSE)) {
 			/*
 			 * More data needed for desegmentation.
 			 */
@@ -1995,6 +2016,37 @@ basic_request_dissector(tvbuff_t *tvb, proto_tree *tree, int offset,
 	    ENC_ASCII|ENC_NA);
 }
 
+static gint
+parse_http_status_code(const guchar *line, const guchar *lineend)
+{
+	const guchar *next_token;
+	int tokenlen;
+	gchar response_code_chars[4];
+	gint32 status_code = 0;
+
+	/*
+	 * The first token is the HTTP Version.
+	 */
+	tokenlen = get_token_len(line, lineend, &next_token);
+	if (tokenlen == 0)
+		return 0;
+	line = next_token;
+
+	/*
+	 * The second token is the Status Code.
+	 */
+	tokenlen = get_token_len(line, lineend, &next_token);
+	if (tokenlen != 3)
+		return 0;
+
+	memcpy(response_code_chars, line, 3);
+	response_code_chars[3] = '\0';
+	if (!ws_strtoi32(response_code_chars, NULL, &status_code))
+		return 0;
+
+	return status_code;
+}
+
 static void
 basic_response_dissector(tvbuff_t *tvb, proto_tree *tree, int offset,
 			 const guchar *line, const guchar *lineend,
diff --git a/epan/dissectors/packet-rtsp.c b/epan/dissectors/packet-rtsp.c
index dc16c3c3fd..8d015d0bde 100644
--- a/epan/dissectors/packet-rtsp.c
+++ b/epan/dissectors/packet-rtsp.c
@@ -844,9 +844,12 @@ dissect_rtspmessage(tvbuff_t *tvb, int offset, packet_info *pinfo,
          * Do header desegmentation if we've been told to,
          * and do body desegmentation if we've been told to and
          * we find a Content-Length header.
+         *
+         * RFC 7826, Section 18.17. requires Content-Length and
+         * assumes zero if missing.
          */
         if (!req_resp_hdrs_do_reassembly(tvb, offset, pinfo,
-            rtsp_desegment_headers, rtsp_desegment_body)) {
+            rtsp_desegment_headers, rtsp_desegment_body, FALSE)) {
             /*
              * More data needed for desegmentation.
              */
diff --git a/epan/dissectors/packet-sip.c b/epan/dissectors/packet-sip.c
index aed92066de..f4560928ad 100644
--- a/epan/dissectors/packet-sip.c
+++ b/epan/dissectors/packet-sip.c
@@ -3408,9 +3408,11 @@ dissect_sip_common(tvbuff_t *tvb, int offset, int remaining_length, packet_info
          * Do header desegmentation if we've been told to,
          * and do body desegmentation if we've been told to and
          * we find a Content-Length header.
+         *
+         * RFC 6594, Section 20.14. requires Content-Length for TCP.
          */
         if (!req_resp_hdrs_do_reassembly(tvb, offset, pinfo,
-            sip_desegment_headers, sip_desegment_body)) {
+            sip_desegment_headers, sip_desegment_body, FALSE)) {
             /*
              * More data needed for desegmentation.
              */
diff --git a/epan/req_resp_hdrs.c b/epan/req_resp_hdrs.c
index d9e3a2fa1e..a3f7c3145d 100644
--- a/epan/req_resp_hdrs.c
+++ b/epan/req_resp_hdrs.c
@@ -26,7 +26,8 @@
  */
 gboolean
 req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const int offset, packet_info *pinfo,
-    const gboolean desegment_headers, const gboolean desegment_body)
+    const gboolean desegment_headers, const gboolean desegment_body,
+    gboolean desegment_until_fin)
 {
 	gint		next_offset;
 	gint		next_offset_sav;
@@ -37,7 +38,6 @@ req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const int offset, packet_info *pinfo,
 	gboolean	content_length_found = FALSE;
 	gboolean	content_type_found = FALSE;
 	gboolean	chunked_encoding = FALSE;
-	gboolean	keepalive_found = FALSE;
 	gchar		*line;
 	gchar		*content_type = NULL;
 
@@ -162,17 +162,6 @@ req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const int offset, packet_info *pinfo,
 					while (*content_type == ' ') {
 						content_type++;
 					}
-				} else if (g_ascii_strncasecmp(line, "Connection:", 11) == 0) {
-					/* Check for keep-alive */
-					header_val = line+11;
-					if(header_val){
-						while(*header_val==' '){
-							header_val++;
-						}
-						if(!g_ascii_strncasecmp(header_val, "Keep-Alive", 10)){
-							keepalive_found = TRUE;
-						}
-					}
 				} else if (g_ascii_strncasecmp( line, "Transfer-Encoding:", 18) == 0) {
 					/*
 					 * Find out if this Transfer-Encoding is
@@ -361,15 +350,16 @@ req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const int offset, packet_info *pinfo,
 				    content_length - length_remaining;
 				return FALSE;
 			}
-		} else if (content_type_found && pinfo->can_desegment) {
-			/* We found a content-type but no content-length.
-			 * This is probably a HTTP header for a session with
-			 * only one HTTP PDU and where the content spans
-			 * until the end of the tcp session, unless there
-			 * is a keepalive header present in which case we
-			 * assume there is no message body at all and thus
-			 * we won't do any reassembly.
-			 * Set up tcp reassembly until the end of this session.
+		} else if (desegment_until_fin && pinfo->can_desegment) {
+			/*
+			 * No Content-Length nor Transfer-Encoding headers are
+			 * found. For HTTP requests, there is definitely no
+			 * body (case 6 of RFC 7230, Section 3.3.3.). For HTTP
+			 * responses, the message body length runs until the end
+			 * of the connection (case 7).
+			 *
+			 * Protocols like RTSP treat absence of Content-Length
+			 * as 0, so do not request more segments either.
 			 */
 			length_remaining = tvb_captured_length_remaining(tvb, next_offset);
 			reported_length_remaining = tvb_reported_length_remaining(tvb, next_offset);
@@ -381,14 +371,6 @@ req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const int offset, packet_info *pinfo,
 				return TRUE;
 			}
 
-			if (keepalive_found) {
-				/* We have a keep-alive but no content-length.
-				 * Assume there is no message body and don't
-				 * do any reassembly.
-				 */
-				return TRUE;
-			}
-
 			pinfo->desegment_offset = offset;
 			pinfo->desegment_len = DESEGMENT_UNTIL_FIN;
 
diff --git a/epan/req_resp_hdrs.h b/epan/req_resp_hdrs.h
index 1bf6374717..f29e970c11 100644
--- a/epan/req_resp_hdrs.h
+++ b/epan/req_resp_hdrs.h
@@ -21,11 +21,15 @@
  *  @param offset   The offset in the buffer to begin inspection.
  *  @param pinfo    Packet info from the parent protocol.
  *  @param desegment_headers    Do desegmentation on headers.
- *  @param desegment_body   Do desegmenation on body.
+ *  @param desegment_body   Do desegmentation on body.
+ *  @param desegment_until_fin  When desegment_body is enabled and no
+ *  Content-Length header is found, assume that all data following the headers
+ *  are part of the body.
  *  @return TRUE if desegmentation is complete otherwise FALSE
  */
 WS_DLL_PUBLIC gboolean
 req_resp_hdrs_do_reassembly(tvbuff_t *tvb, const  int offset, packet_info *pinfo,
-    const gboolean desegment_headers, const gboolean desegment_body);
+    const gboolean desegment_headers, const gboolean desegment_body,
+    gboolean desegment_until_fin);
 
 #endif