From: Mats Klepsland Date: Thu, 27 May 2021 10:02:55 +0000 (+0200) Subject: thresholds: Fix buffer overflow in threshold context X-Git-Tag: suricata-6.0.3~32 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ff75718eee0e58ce21c8cea4938d797691de1a8;p=thirdparty%2Fsuricata.git thresholds: Fix buffer overflow in threshold context th_entry is resized using ThresholdHashRealloc() every time a rule with a threshold using by_rule tracking is added. The problem is that this is done before the rules are reordered, so occasionally a rule with by_rule tracking gets a higher signature number (after reordering) than the number of th_entries allocated, causing Suricata to crash. This commit fixes this by allocating th_entries after all the rules are loaded and reordered. Backtrace from core dump: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x000000000051b381 in ThresholdHandlePacket (p=p@entry=0x7fb0080f3960, lookup_tsh=0x51, new_tsh=new_tsh@entry=0x7fb016c316e0, td=td@entry=0x14adedf0, sid=9800979, gid=1, pa=0x7fb0080f3b18) at detect-engine-threshold.c:415 415>---- if (TIMEVAL_DIFF_SEC(p->ts, lookup_tsh->tv1) < td->seconds) { Bug #4503. (cherry picked from commit 2a326421aa29154ebfaada3888974a634feb5f56) --- diff --git a/src/detect-engine-build.c b/src/detect-engine-build.c index 784337b099..ecd21f8684 100644 --- a/src/detect-engine-build.c +++ b/src/detect-engine-build.c @@ -28,6 +28,7 @@ #include "detect-engine-port.h" #include "detect-engine-prefilter.h" #include "detect-engine-proto.h" +#include "detect-engine-threshold.h" #include "detect-dsize.h" #include "detect-tcp-flags.h" @@ -1944,6 +1945,8 @@ int SigGroupBuild(DetectEngineCtx *de_ctx) SCProfilingRuleInitCounters(de_ctx); #endif + ThresholdHashAllocate(de_ctx); + if (!DetectEngineMultiTenantEnabled()) { VarNameStoreActivateStaging(); } diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c index d98931de95..355e1dc35d 100644 --- a/src/detect-engine-threshold.c +++ b/src/detect-engine-threshold.c @@ -651,27 +651,71 @@ void ThresholdHashInit(DetectEngineCtx *de_ctx) } /** - * \brief Realloc threshold context hash tables + * \brief Allocate threshold context hash tables * * \param de_ctx Detection Context */ -void ThresholdHashRealloc(DetectEngineCtx *de_ctx) +void ThresholdHashAllocate(DetectEngineCtx *de_ctx) { - /* Return if we are already big enough */ - uint32_t num = de_ctx->signum + 1; - if (num <= de_ctx->ths_ctx.th_size) - return; + Signature *s = de_ctx->sig_list; + bool has_by_rule_tracking = false; + const DetectThresholdData *td = NULL; + const SigMatchData *smd; + + /* Find the signature with the highest signature number that is using + thresholding with by_rule tracking. */ + uint32_t highest_signum = 0; + while (s != NULL) { + if (s->sm_arrays[DETECT_SM_LIST_SUPPRESS] != NULL) { + smd = NULL; + do { + td = SigGetThresholdTypeIter(s, &smd, DETECT_SM_LIST_SUPPRESS); + if (td == NULL) { + continue; + } + if (td->track != TRACK_RULE) { + continue; + } + if (s->num >= highest_signum) { + highest_signum = s->num; + has_by_rule_tracking = true; + } + } while (smd != NULL); + } - void *ptmp = SCRealloc(de_ctx->ths_ctx.th_entry, num * sizeof(DetectThresholdEntry *)); - if (ptmp == NULL) { - SCLogWarning(SC_ERR_MEM_ALLOC, "Error allocating memory for rule thresholds" - " (tried to allocate %"PRIu32" th_entrys for rule tracking)", num); - } else { - de_ctx->ths_ctx.th_entry = ptmp; - for (uint32_t i = de_ctx->ths_ctx.th_size; i < num; ++i) { - de_ctx->ths_ctx.th_entry[i] = NULL; + if (s->sm_arrays[DETECT_SM_LIST_THRESHOLD] != NULL) { + smd = NULL; + do { + td = SigGetThresholdTypeIter(s, &smd, DETECT_SM_LIST_THRESHOLD); + if (td == NULL) { + continue; + } + if (td->track != TRACK_RULE) { + continue; + } + if (s->num >= highest_signum) { + highest_signum = s->num; + has_by_rule_tracking = true; + } + } while (smd != NULL); } - de_ctx->ths_ctx.th_size = num; + + s = s->next; + } + + /* Skip allocating if by_rule tracking is not used */ + if (has_by_rule_tracking == false) { + return; + } + + de_ctx->ths_ctx.th_size = highest_signum + 1; + de_ctx->ths_ctx.th_entry = SCCalloc(de_ctx->ths_ctx.th_size, sizeof(DetectThresholdEntry *)); + if (de_ctx->ths_ctx.th_entry == NULL) { + FatalError(SC_ERR_MEM_ALLOC, + "Error allocating memory for rule " + "thresholds (tried to allocate %" PRIu32 " th_entrys for " + "rule tracking)", + de_ctx->ths_ctx.th_size); } } diff --git a/src/detect-engine-threshold.h b/src/detect-engine-threshold.h index e3115b873f..d94d8ec6ad 100644 --- a/src/detect-engine-threshold.h +++ b/src/detect-engine-threshold.h @@ -43,7 +43,7 @@ int PacketAlertThreshold(DetectEngineCtx *, DetectEngineThreadCtx *, const Signature *, PacketAlert *); void ThresholdHashInit(DetectEngineCtx *); -void ThresholdHashRealloc(DetectEngineCtx *); +void ThresholdHashAllocate(DetectEngineCtx *); void ThresholdContextDestroy(DetectEngineCtx *); int ThresholdHostTimeoutCheck(Host *, struct timeval *); diff --git a/src/detect-threshold.c b/src/detect-threshold.c index 3abe8d263c..baf8aea550 100644 --- a/src/detect-threshold.c +++ b/src/detect-threshold.c @@ -251,9 +251,6 @@ static int DetectThresholdSetup(DetectEngineCtx *de_ctx, Signature *s, const cha if (de == NULL) goto error; - if (de->track == TRACK_RULE) - ThresholdHashRealloc(de_ctx); - sm = SigMatchAlloc(); if (sm == NULL) goto error; @@ -1604,7 +1601,6 @@ static int DetectThresholdTestSig13(void) SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); - ThresholdHashRealloc(de_ctx); /* should alert twice */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p); diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index 9d4bfb177e..104ccd663a 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -487,9 +487,6 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid sm->type = DETECT_THRESHOLD; sm->ctx = (void *)de; - if (parsed_track == TRACK_RULE) { - ThresholdHashRealloc(de_ctx); - } SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_THRESHOLD); } @@ -530,9 +527,6 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid sm->type = DETECT_THRESHOLD; sm->ctx = (void *)de; - if (parsed_track == TRACK_RULE) { - ThresholdHashRealloc(de_ctx); - } SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_THRESHOLD); } } @@ -604,10 +598,6 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid sm->type = DETECT_THRESHOLD; sm->ctx = (void *)de; - if (parsed_track == TRACK_RULE) { - ThresholdHashRealloc(de_ctx); - } - SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_THRESHOLD); } }