]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
app-layer: Set sc_errno upon error return
authorRichard McConnell <Richard_McConnell@rapid7.com>
Thu, 16 May 2024 09:04:24 +0000 (10:04 +0100)
committerVictor Julien <victor@inliniac.net>
Wed, 22 May 2024 18:18:59 +0000 (20:18 +0200)
Bug: https://redmine.openinfosecfoundation.org/issues/6782

Callers to these allocators often use ``sc_errno`` to provide context of
the error. And in the case of the above bug, they return ``sc_errno``,
but as it has not been set ``sc_errno = 0; == SC_OK``.

This patch simply sets this variable to ensure there is context provided
upon error.

src/app-layer-ftp.c
src/app-layer-htp-mem.c
src/util-streaming-buffer.c

index d2777198ab4e75aff68e12a1f6fb6835b9fad118..7384e732e53376098d6cd53550a7025eebc4e03e 100644 (file)
@@ -192,13 +192,17 @@ static int FTPCheckMemcap(uint64_t size)
 
 static void *FTPCalloc(size_t n, size_t size)
 {
-    if (FTPCheckMemcap((uint32_t)(n * size)) == 0)
+    if (FTPCheckMemcap((uint32_t)(n * size)) == 0) {
+        sc_errno = SC_ELIMIT;
         return NULL;
+    }
 
     void *ptr = SCCalloc(n, size);
 
-    if (unlikely(ptr == NULL))
+    if (unlikely(ptr == NULL)) {
+        sc_errno = SC_ENOMEM;
         return NULL;
+    }
 
     FTPIncrMemuse((uint64_t)(n * size));
     return ptr;
@@ -208,12 +212,16 @@ static void *FTPRealloc(void *ptr, size_t orig_size, size_t size)
 {
     void *rptr = NULL;
 
-    if (FTPCheckMemcap((uint32_t)(size - orig_size)) == 0)
+    if (FTPCheckMemcap((uint32_t)(size - orig_size)) == 0) {
+        sc_errno = SC_ELIMIT;
         return NULL;
+    }
 
     rptr = SCRealloc(ptr, size);
-    if (rptr == NULL)
+    if (rptr == NULL) {
+        sc_errno = SC_ENOMEM;
         return NULL;
+    }
 
     if (size > orig_size) {
         FTPIncrMemuse(size - orig_size);
index bd9b79f6762398435a40a151ad1709c6b811c38b..57967b1d6e4e2f22b042997d52a2caf674b30e12 100644 (file)
@@ -136,13 +136,17 @@ void *HTPMalloc(size_t size)
 {
     void *ptr = NULL;
 
-    if (HTPCheckMemcap((uint32_t)size) == 0)
+    if (HTPCheckMemcap((uint32_t)size) == 0) {
+        sc_errno = SC_ELIMIT;
         return NULL;
+    }
 
     ptr = SCMalloc(size);
 
-    if (unlikely(ptr == NULL))
+    if (unlikely(ptr == NULL)) {
+        sc_errno = SC_ENOMEM;
         return NULL;
+    }
 
     HTPIncrMemuse((uint64_t)size);
 
@@ -153,13 +157,17 @@ void *HTPCalloc(size_t n, size_t size)
 {
     void *ptr = NULL;
 
-    if (HTPCheckMemcap((uint32_t)(n * size)) == 0)
+    if (HTPCheckMemcap((uint32_t)(n * size)) == 0) {
+        sc_errno = SC_ELIMIT;
         return NULL;
+    }
 
     ptr = SCCalloc(n, size);
 
-    if (unlikely(ptr == NULL))
+    if (unlikely(ptr == NULL)) {
+        sc_errno = SC_ENOMEM;
         return NULL;
+    }
 
     HTPIncrMemuse((uint64_t)(n * size));
 
@@ -169,13 +177,17 @@ void *HTPCalloc(size_t n, size_t size)
 void *HTPRealloc(void *ptr, size_t orig_size, size_t size)
 {
     if (size > orig_size) {
-        if (HTPCheckMemcap((uint32_t)(size - orig_size)) == 0)
+        if (HTPCheckMemcap((uint32_t)(size - orig_size)) == 0) {
+            sc_errno = SC_ELIMIT;
             return NULL;
+        }
     }
 
     void *rptr = SCRealloc(ptr, size);
-    if (rptr == NULL)
+    if (rptr == NULL) {
+        sc_errno = SC_ENOMEM;
         return NULL;
+    }
 
     if (size > orig_size) {
         HTPIncrMemuse((uint64_t)(size - orig_size));
index 7ef6f58e2c35f6146a69fbf31edcaf1da601cf0d..69d865370118e94e43a88e97b69019d91169f9da 100644 (file)
 #include "util-debug.h"
 #include "util-error.h"
 
+#include "app-layer-htp-mem.h"
+#include "conf-yaml-loader.h"
+#include "app-layer-htp.h"
+
 static void ListRegions(StreamingBuffer *sb);
 
 #define DUMP_REGIONS 0 // set to 1 to dump a visual representation of the regions list and sbb tree.
@@ -721,6 +725,8 @@ static inline int WARN_UNUSED GrowRegionToSize(StreamingBuffer *sb,
 
     void *ptr = REALLOC(cfg, region->buf, region->buf_size, grow);
     if (ptr == NULL) {
+        if (sc_errno == SC_OK)
+            sc_errno = SC_ENOMEM;
         return sc_errno;
     }
     /* for safe printing and general caution, lets memset the
@@ -1099,6 +1105,8 @@ int StreamingBufferAppend(StreamingBuffer *sb, const StreamingBufferConfig *cfg,
         }
     }
     DEBUG_VALIDATE_BUG_ON(DataFits(sb, data_len) != 1);
+    if (DataFits(sb, data_len) != 1)
+        return -1;
 
     memcpy(sb->region.buf + sb->region.buf_offset, data, data_len);
     seg->stream_offset = sb->region.stream_offset + sb->region.buf_offset;
@@ -2366,6 +2374,45 @@ static int StreamingBufferTest11(void)
     StreamingBufferFree(sb, &cfg);
     PASS;
 }
+
+static const char *dummy_conf_string = "%YAML 1.1\n"
+                                       "---\n"
+                                       "\n"
+                                       "app-layer:\n"
+                                       "  protocols:\n"
+                                       "    http:\n"
+                                       "      enabled: yes\n"
+                                       "      memcap: 88\n"
+                                       "\n";
+
+static int StreamingBufferTest12(void)
+{
+    ConfCreateContextBackup();
+    ConfInit();
+    HtpConfigCreateBackup();
+    ConfYamlLoadString((const char *)dummy_conf_string, strlen(dummy_conf_string));
+    HTPConfigure();
+
+    StreamingBufferConfig cfg = { 8, 1, STREAMING_BUFFER_REGION_GAP_DEFAULT, HTPCalloc, HTPRealloc,
+        HTPFree };
+    StreamingBuffer *sb = StreamingBufferInit(&cfg);
+    FAIL_IF(sb == NULL);
+
+    StreamingBufferSegment seg1;
+    FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg1, (const uint8_t *)"ABCDEFGHIJKLMNOP", 16) != 0);
+
+    StreamingBufferSegment seg2;
+    FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg2,
+                    (const uint8_t *)"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ",
+                    52) != -1);
+    FAIL_IF(sc_errno != SC_ELIMIT);
+
+    StreamingBufferFree(sb, &cfg);
+    HtpConfigRestoreBackup();
+    ConfRestoreContextBackup();
+
+    PASS;
+}
 #endif
 
 void StreamingBufferRegisterTests(void)
@@ -2380,5 +2427,6 @@ void StreamingBufferRegisterTests(void)
     UtRegisterTest("StreamingBufferTest09", StreamingBufferTest09);
     UtRegisterTest("StreamingBufferTest10", StreamingBufferTest10);
     UtRegisterTest("StreamingBufferTest11 Bug 6903", StreamingBufferTest11);
+    UtRegisterTest("StreamingBufferTest12 Bug 6782", StreamingBufferTest12);
 #endif
 }