From: Fupeng Zhao Date: Thu, 19 Jun 2025 06:03:50 +0000 (+0800) Subject: util/coredump: refactor parsing and respect zero core dump limit X-Git-Tag: suricata-8.0.0~21 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d618585c4f2fd97a49a57a2fbf626c3ae08cfab1;p=thirdparty%2Fsuricata.git 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 d4e57e9fe6..69a37788b8 100644 --- 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 c35ba37595..747451f85e 100644 --- a/src/util-coredump-config.c +++ b/src/util-coredump-config.c @@ -32,6 +32,7 @@ #ifdef HAVE_SYS_PRCTL_H #include #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 c5e01f4d3e..e5dc583f51 100644 --- 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 */