util/coredump: refactor parsing and respect zero core dump limit

- Replaced strtoul/strtoull with ByteExtractString* for safer and more consistent parsing.
- Allowed max-dump to be set to 0, and correctly apply a core dump limit of 0, maintaining behavior consistent with the commented default in suricata.yaml.in.
- Added and registered unit tests to validate the updated logic.

Ticket: #7212

diff --git a/src/runmode-unittests.c b/src/runmode-unittests.c
index d4e57e9fe62ce0b35922bb81405d922c0364fa00..69a37788b8a2533f51236ec96d76a0f1e5d2b5a1 100644 (file)
--- a/src/runmode-unittests.c
+++ b/src/runmode-unittests.c
@@ -68,6 +68,7 @@
 #include "util-radix6-tree.h"
 #include "util-host-os-info.h"
 #include "util-cidr.h"
+#include "util-coredump-config.h"
 #include "util-unittest-helper.h"
 #include "util-time.h"
 #include "util-rule-vars.h"
@@ -218,6 +219,7 @@ static void RegisterUnittests(void)
     SCProtoNameRegisterTests();
     UtilCIDRTests();
     OutputJsonStatsRegisterTests();
+    CoredumpConfigRegisterTests();
 }
 #endif
 

diff --git a/src/util-coredump-config.c b/src/util-coredump-config.c
index c35ba37595916a08a1e1bb4820b7b078efcc8974..747451f85e9d99bc34b0cf12fedfd47c61e14084 100644 (file)
--- a/src/util-coredump-config.c
+++ b/src/util-coredump-config.c
@@ -32,6 +32,7 @@
 #ifdef HAVE_SYS_PRCTL_H
 #include <sys/prctl.h>
 #endif
+#include "util-byte.h"
 #include "util-debug.h"
 
 #ifdef OS_WIN32
@@ -45,6 +46,10 @@ int32_t CoredumpLoadConfig(void) {
     return 0;
 }
 
+void CoredumpConfigRegisterTests(void)
+{
+}
+
 #else
 
 static bool unlimited = false;
@@ -91,6 +96,9 @@ int32_t CoredumpLoadConfig (void)
 {
 #ifdef HAVE_SYS_RESOURCE_H
     /* get core dump configuration settings for suricata */
+    int ret = 0;
+    uint64_t max_dump64 = 0;
+    uint32_t max_dump32 = 0;
     const char *dump_size_config = NULL;
     size_t rlim_size = sizeof(rlim_t);
 
@@ -116,18 +124,25 @@ int32_t CoredumpLoadConfig (void)
             SCLogInfo ("Unexpected type for rlim_t");
             return 0;
         }
-        errno = 0;
         if (rlim_size == 8) {
-            max_dump = (rlim_t) strtoull (dump_size_config, NULL, 10);
+            ret = ByteExtractStringUint64(
+                    &max_dump64, 10, strlen(dump_size_config), dump_size_config);
         }
         else if (rlim_size == 4) {
-            max_dump = (rlim_t) strtoul (dump_size_config, NULL, 10);
+            ret = ByteExtractStringUint32(
+                    &max_dump32, 10, strlen(dump_size_config), dump_size_config);
         }
-        if ((errno == ERANGE) || (errno != 0 && max_dump == 0)) {
+        if (ret <= 0) {
             SCLogInfo ("Illegal core dump size: %s.", dump_size_config);
             return 0;
         }
-        SCLogInfo ("Max dump is %"PRIu64, (uint64_t) max_dump);
+
+        max_dump = rlim_size == 8 ? (rlim_t)max_dump64 : (rlim_t)max_dump32;
+        if (max_dump == 0) {
+            SCLogInfo("Max dump is 0, disable core dumping.");
+        } else {
+            SCLogInfo("Max dump is %" PRIu64, (uint64_t)max_dump);
+        }
     }
 
     CoredumpEnable();
@@ -238,4 +253,172 @@ int32_t CoredumpLoadConfig (void)
     return 0;
 }
 
+#if defined UNITTESTS && defined HAVE_SYS_RESOURCE_H
+#include "conf-yaml-loader.h"
+
+static void ResetCoredumpConfig(void)
+{
+    unlimited = false;
+    max_dump = 0;
+}
+
+/**
+ * \test CoredumpConfigTest01 is a test for the coredump configuration
+ *        with unlimited coredump size.
+ */
+static int CoredumpConfigTest01(void)
+{
+    char config[] = "\
+%YAML 1.1\n\
+---\n\
+coredump:\n\
+  max-dump: unlimited\n\
+";
+
+    int ret = 0;
+    ResetCoredumpConfig();
+    SCConfCreateContextBackup();
+    SCConfInit();
+
+    SCConfYamlLoadString(config, strlen(config));
+    ret = CoredumpLoadConfig();
+    FAIL_IF(errno != EPERM && ret != 1);
+
+    FAIL_IF(unlimited != true);
+    FAIL_IF(max_dump != 0);
+
+    SCConfDeInit();
+    SCConfRestoreContextBackup();
+    PASS;
+}
+
+/**
+ * \test CoredumpConfigTest02 is a test for the coredump configuration
+ *        with a specific coredump size.
+ */
+static int CoredumpConfigTest02(void)
+{
+    char config[] = "\
+%YAML 1.1\n\
+---\n\
+coredump:\n\
+  max-dump: 1000000\n\
+";
+
+    int ret = 0;
+    ResetCoredumpConfig();
+    SCConfCreateContextBackup();
+    SCConfInit();
+
+    SCConfYamlLoadString(config, strlen(config));
+    ret = CoredumpLoadConfig();
+    FAIL_IF(errno != EPERM && ret != 1);
+
+    FAIL_IF(unlimited != false);
+    FAIL_IF(max_dump != 1000000);
+
+    SCConfDeInit();
+    SCConfRestoreContextBackup();
+    PASS;
+}
+
+/**
+ * \test CoredumpConfigTest03 is a test for the coredump configuration
+ *        with an invalid coredump size.
+ */
+static int CoredumpConfigTest03(void)
+{
+    char config[] = "\
+%YAML 1.1\n\
+---\n\
+coredump:\n\
+  max-dump: -1000000\n\
+";
+
+    ResetCoredumpConfig();
+    SCConfCreateContextBackup();
+    SCConfInit();
+
+    SCConfYamlLoadString(config, strlen(config));
+    FAIL_IF(CoredumpLoadConfig() != 0);
+
+    FAIL_IF(unlimited != false);
+    FAIL_IF(max_dump != 0);
+
+    SCConfDeInit();
+    SCConfRestoreContextBackup();
+    PASS;
+}
+
+/**
+ * \test CoredumpConfigTest04 is a test for the coredump configuration
+ *        with a non-numeric coredump size.
+ */
+static int CoredumpConfigTest04(void)
+{
+    char config[] = "\
+%YAML 1.1\n\
+---\n\
+coredump:\n\
+  max-dump: a\n\
+";
+
+    ResetCoredumpConfig();
+    SCConfCreateContextBackup();
+    SCConfInit();
+
+    SCConfYamlLoadString(config, strlen(config));
+    FAIL_IF(CoredumpLoadConfig() != 0);
+
+    FAIL_IF(unlimited != false);
+    FAIL_IF(max_dump != 0);
+
+    SCConfDeInit();
+    SCConfRestoreContextBackup();
+    PASS;
+}
+
+/**
+ * \test CoredumpConfigTest05 is a test for the coredump configuration
+ *        with a zero coredump size.
+ */
+static int CoredumpConfigTest05(void)
+{
+    char config[] = "\
+%YAML 1.1\n\
+---\n\
+coredump:\n\
+  max-dump: 0\n\
+";
+
+    int ret = 0;
+    ResetCoredumpConfig();
+    SCConfCreateContextBackup();
+    SCConfInit();
+
+    SCConfYamlLoadString(config, strlen(config));
+    ret = CoredumpLoadConfig();
+    /* should return 1 because zero coredump size is allowed. */
+    FAIL_IF(errno != EPERM && ret != 1);
+
+    FAIL_IF(unlimited != false);
+    FAIL_IF(max_dump != 0);
+
+    SCConfDeInit();
+    SCConfRestoreContextBackup();
+    PASS;
+}
+#endif /* UNITTESTS */
+
+void CoredumpConfigRegisterTests(void)
+{
+#if defined UNITTESTS && defined HAVE_SYS_RESOURCE_H
+    UtRegisterTest("CoredumpConfigTest01", CoredumpConfigTest01);
+    UtRegisterTest("CoredumpConfigTest02", CoredumpConfigTest02);
+    UtRegisterTest("CoredumpConfigTest03", CoredumpConfigTest03);
+    UtRegisterTest("CoredumpConfigTest04", CoredumpConfigTest04);
+    UtRegisterTest("CoredumpConfigTest05", CoredumpConfigTest05);
+#endif /* UNITTESTS */
+}
+
 #endif /* OS_WIN32 */

diff --git a/src/util-coredump-config.h b/src/util-coredump-config.h
index c5e01f4d3e4c30452711bfa1ebda76d76acf55a8..e5dc583f51938658cdf122b60a01b986236b299e 100644 (file)
--- a/src/util-coredump-config.h
+++ b/src/util-coredump-config.h
@@ -29,4 +29,6 @@
 int32_t CoredumpLoadConfig(void);
 void CoredumpEnable(void);
 
+void CoredumpConfigRegisterTests(void);
+
 #endif /* SURICATA_COREDUMP_CONFIG_H */