From: Jeff Lucovsky Date: Sun, 28 Feb 2021 19:33:58 +0000 (-0500) Subject: thresholds: Improve validation of threshold.config X-Git-Tag: suricata-7.0.0-beta1~1715 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=11ec61d0b481c3d5c83fc6f397acaf1b2b37ff70;p=thirdparty%2Fsuricata.git thresholds: Improve validation of threshold.config This commit improves the handling of threshold.config. When used with "-T", a non-zero return code occurs when the file cannot be validated. To maintain legacy behavior, when "-T" is not used and threshold.config contains one or more invalid lines, Suricata continues execution. --- diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index 271c31e950..4c2f4e58d4 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2015 Open Information Security Foundation +/* Copyright (C) 2021 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -364,7 +364,10 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, int sig_file_excl SCSigOrderSignatures(de_ctx); SCSigSignatureOrderingModuleCleanup(de_ctx); - SCThresholdConfInitContext(de_ctx); + if (SCThresholdConfInitContext(de_ctx) < 0) { + ret = -1; + goto end; + } /* Setup the signature group lookup structure and pattern matchers */ if (SigGroupBuild(de_ctx) < 0) diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index b08935db06..60c3eb9a83 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2016 Open Information Security Foundation +/* Copyright (C) 2007-2021 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -238,6 +238,7 @@ static const char *SCThresholdConfGetConfFilename(const DetectEngineCtx *de_ctx) int SCThresholdConfInitContext(DetectEngineCtx *de_ctx) { const char *filename = NULL; + int ret = 0; #ifndef UNITTESTS FILE *fd = NULL; #else @@ -253,7 +254,15 @@ int SCThresholdConfInitContext(DetectEngineCtx *de_ctx) } #endif - SCThresholdConfParseFile(de_ctx, fd); + if (SCThresholdConfParseFile(de_ctx, fd) < 0) { + SCLogWarning( + SC_WARN_THRESH_CONFIG, "Error loading threshold configuration from %s", filename); + /* maintain legacy behavior so no errors unless config testing */ + if (RunmodeGetCurrent() == RUNMODE_CONF_TEST) { + ret = -1; + } + goto error; + } SCThresholdConfDeInitContext(de_ctx, fd); #ifdef UNITTESTS @@ -264,7 +273,8 @@ int SCThresholdConfInitContext(DetectEngineCtx *de_ctx) error: SCThresholdConfDeInitContext(de_ctx, fd); - return -1; + +return ret; } /** @@ -1064,7 +1074,7 @@ static int SCThresholdConfLineIsMultiline(char *line) * \param de_ctx Pointer to the Detection Engine Context. * \param fd Pointer to file descriptor. */ -void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp) +int SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp) { char line[8192] = ""; int rule_num = 0; @@ -1073,7 +1083,7 @@ void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp) int esc_pos = 0; if (fp == NULL) - return; + return -1; while (fgets(line + esc_pos, (int)sizeof(line) - esc_pos, fp) != NULL) { if (SCThresholdConfIsLineBlankOrComment(line)) { @@ -1082,15 +1092,19 @@ void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp) esc_pos = SCThresholdConfLineIsMultiline(line); if (esc_pos == 0) { - rule_num++; - SCLogDebug("Adding threshold.config rule num %"PRIu32"( %s )", rule_num, line); - SCThresholdConfAddThresholdtype(line, de_ctx); + if (SCThresholdConfAddThresholdtype(line, de_ctx) < 0) { + if (RunmodeGetCurrent() == RUNMODE_CONF_TEST) + return -1; + } else { + SCLogDebug("Adding threshold.config rule num %" PRIu32 "( %s )", rule_num, line); + rule_num++; + } } } SCLogInfo("Threshold config parsed: %d rule(s) found", rule_num); - return; + return 0; } #ifdef UNITTESTS @@ -1334,7 +1348,7 @@ static int SCThresholdConfTest01(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); @@ -1367,7 +1381,7 @@ static int SCThresholdConfTest02(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); @@ -1400,7 +1414,7 @@ static int SCThresholdConfTest03(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); @@ -1433,7 +1447,7 @@ static int SCThresholdConfTest04(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateInValidDummyFD02(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); @@ -1469,7 +1483,7 @@ static int SCThresholdConfTest05(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD03(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); Signature *s = de_ctx->sig_list; SigMatch *m = DetectGetLastSMByListId(s, DETECT_SM_LIST_THRESHOLD, @@ -1517,7 +1531,7 @@ static int SCThresholdConfTest06(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD04(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); @@ -1550,7 +1564,7 @@ static int SCThresholdConfTest07(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD05(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_DETECTION_FILTER, -1); @@ -1584,7 +1598,7 @@ static int SCThresholdConfTest08(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD06(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD, DETECT_DETECTION_FILTER, -1); @@ -1631,7 +1645,7 @@ static int SCThresholdConfTest09(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD07(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -1725,7 +1739,7 @@ static int SCThresholdConfTest10(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD08(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -1818,7 +1832,7 @@ static int SCThresholdConfTest11(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD09(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -1927,7 +1941,7 @@ static int SCThresholdConfTest12(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD10(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2017,7 +2031,7 @@ static int SCThresholdConfTest13(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_SUPPRESS, DETECT_THRESHOLD, -1); @@ -2073,7 +2087,7 @@ static int SCThresholdConfTest14(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2129,7 +2143,7 @@ static int SCThresholdConfTest15(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2181,7 +2195,7 @@ static int SCThresholdConfTest16(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2232,7 +2246,7 @@ static int SCThresholdConfTest17(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2289,7 +2303,7 @@ static int SCThresholdConfTest18(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateInvalidDummyFD12(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]); @@ -2340,7 +2354,7 @@ static int SCThresholdConfTest19(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateInvalidDummyFD13(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]); SigMatchData *smd = s->sm_arrays[DETECT_SM_LIST_SUPPRESS]; @@ -2390,7 +2404,7 @@ static int SCThresholdConfTest20(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD20(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]); @@ -2435,7 +2449,7 @@ static int SCThresholdConfTest21(void) FAIL_IF_NULL(s); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD20(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]); @@ -2522,7 +2536,7 @@ static int SCThresholdConfTest22(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD22(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -2658,7 +2672,7 @@ static int SCThresholdConfTest23(void) FAIL_IF_NOT_NULL(g_ut_threshold_fp); g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD23(); FAIL_IF_NULL(g_ut_threshold_fp); - SCThresholdConfInitContext(de_ctx); + FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx)); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); diff --git a/src/util-threshold-config.h b/src/util-threshold-config.h index aec87d2152..982bf3e1c9 100644 --- a/src/util-threshold-config.h +++ b/src/util-threshold-config.h @@ -24,7 +24,7 @@ #ifndef __UTIL_THRESHOLD_CONFIG_H__ #define __UTIL_THRESHOLD_CONFIG_H__ -void SCThresholdConfParseFile(DetectEngineCtx *, FILE *); +int SCThresholdConfParseFile(DetectEngineCtx *, FILE *); int SCThresholdConfInitContext(DetectEngineCtx *); void SCThresholdConfRegisterTests(void);