From: Victor Julien <victor@inliniac.net>
Date: Wed, 29 Mar 2017 09:15:51 +0000 (+0200)
Subject: http: fix body tracking corner case
X-Git-Tag: suricata-4.0.0-beta1~239
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F2629%2Fhead;p=thirdparty%2Fsuricata.git

http: fix body tracking corner case

In some cases, observed with inspect limits 0, the body tracking could
get confused. When all chunks were already freed, a new chunk would
be considered to be the start of the body. This would overwrite the
bodies 'content_len_so_far' tracker, instead of adding to it. This in
turn could lead to a assertion abort in the inspection code.

This patch redoes the append code to always add the current lenght. It
cleans up the code to remove redundant logic.

Issue: https://redmine.openinfosecfoundation.org/issues/2078
Reported-By: Jørgen Bøhnsdalen
---

diff --git a/src/app-layer-htp-body.c b/src/app-layer-htp-body.c
index dfda083b6b..3520182a63 100644
--- a/src/app-layer-htp-body.c
+++ b/src/app-layer-htp-body.c
@@ -93,37 +93,25 @@ int HtpBodyAppendChunk(const HTPCfgDir *hcfg, HtpBody *body,
             SCReturnInt(-1);
     }
 
-    if (body->first == NULL) {
-        /* New chunk */
-        bd = (HtpBodyChunk *)HTPCalloc(1, sizeof(HtpBodyChunk));
-        if (bd == NULL) {
-            SCReturnInt(-1);
-        }
+    /* New chunk */
+    bd = (HtpBodyChunk *)HTPCalloc(1, sizeof(HtpBodyChunk));
+    if (bd == NULL) {
+        SCReturnInt(-1);
+    }
 
-        if (StreamingBufferAppend(body->sb, &bd->sbseg, data, len) != 0) {
-            HTPFree(bd, sizeof(HtpBodyChunk));
-            SCReturnInt(-1);
-        }
+    if (StreamingBufferAppend(body->sb, &bd->sbseg, data, len) != 0) {
+        HTPFree(bd, sizeof(HtpBodyChunk));
+        SCReturnInt(-1);
+    }
 
+    if (body->first == NULL) {
         body->first = body->last = bd;
-
-        body->content_len_so_far = len;
     } else {
-        bd = (HtpBodyChunk *)HTPCalloc(1, sizeof(HtpBodyChunk));
-        if (bd == NULL) {
-            SCReturnInt(-1);
-        }
-
-        if (StreamingBufferAppend(body->sb, &bd->sbseg, data, len) != 0) {
-            HTPFree(bd, sizeof(HtpBodyChunk));
-            SCReturnInt(-1);
-        }
-
         body->last->next = bd;
         body->last = bd;
-
-        body->content_len_so_far += len;
     }
+    body->content_len_so_far += len;
+
     SCLogDebug("body %p", body);
 
     SCReturnInt(0);