From: Philippe Antoine <contact@catenacyber.fr>
Date: Tue, 7 Sep 2021 12:52:36 +0000 (+0200)
Subject: range: post process out of order blocks
X-Git-Tag: suricata-7.0.0-beta1~1353
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bded2ec92e13699ca4a101bbbe315334ab97751f;p=thirdparty%2Fsuricata.git

range: post process out of order blocks

A block is determined out of order on opening.
But on closing, the gap before it may have been filled.
So, we must post-process it, ie iterate over the red and black
tree so see what blocks we can get.
---

diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c
index 7f6ecf8079..0c508f1cf2 100644
--- a/src/app-layer-htp-range.c
+++ b/src/app-layer-htp-range.c
@@ -445,6 +445,7 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags)
             SCFree(c->current);
             c->current = NULL;
             SCLogDebug("c->current was obsolete");
+            return NULL;
         } else {
             /* otherwise insert in red and black tree. If res != NULL, the insert
                failed because its a dup. */
@@ -458,24 +459,27 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags)
             }
             SCLogDebug("inserted range fragment");
             c->current = NULL;
+            if (c->container->files == NULL) {
+                // we have to wait for the flow owning the file
+                return NULL;
+            }
+            // keep on going, maybe this out of order chunk
+            // became the missing part between open and close
         }
         SCLogDebug("c->current was set, file incomplete so return NULL");
-        return NULL;
-    }
-
-    if (c->toskip > 0) {
+    } else if (c->toskip > 0) {
         // was only an overlapping range, truncated before new bytes
         SCLogDebug("c->toskip %" PRIu64, c->toskip);
         return NULL;
+    } else {
+        // we just finished an in-order block
+        DEBUG_VALIDATE_BUG_ON(c->files == NULL);
+        // move back the ownership of the file container to HttpRangeContainerFile
+        c->container->files = c->files;
+        c->files = NULL;
+        DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL);
     }
 
-    // we just finished an in-order block
-    DEBUG_VALIDATE_BUG_ON(c->files == NULL);
-    // move back the ownership of the file container to HttpRangeContainerFile
-    c->container->files = c->files;
-    c->files = NULL;
-    DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL);
-    // we update the file size now that we own it again
     File *f = c->container->files->tail;
 
     /* See if we can use our stored fragments to (partly) reconstruct the file */