From 05eac977f07e856f9f7fc9dce94ebcc2672b604d Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:57:18 +0200 Subject: [PATCH] detect/sigorder: handle allocation failure Addresses a gcc -fanalyzer warning. --- src/detect-engine-loader.c | 5 ++++- src/detect-engine-sigorder.c | 35 ++++++++++++++++++++++------------- src/detect-engine-sigorder.h | 2 +- src/detect-flowint.c | 6 +++--- src/util-unittest-helper.c | 4 +++- 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index ca9cda0ce0..038146a946 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -500,7 +500,10 @@ skip_regular_rules: } SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + ret = -1; + goto end; + } SCSigSignatureOrderingModuleCleanup(de_ctx); if (SCThresholdConfInitContext(de_ctx) < 0) { diff --git a/src/detect-engine-sigorder.c b/src/detect-engine-sigorder.c index 9447b52739..98012cf9d3 100644 --- a/src/detect-engine-sigorder.c +++ b/src/detect-engine-sigorder.c @@ -799,13 +799,14 @@ static inline SCSigSignatureWrapper *SCSigAllocSignatureWrapper(Signature *sig) * \param de_ctx Pointer to the Detection Engine Context that holds the * signatures to be ordered */ -void SCSigOrderSignatures(DetectEngineCtx *de_ctx) +int SCSigOrderSignatures(DetectEngineCtx *de_ctx) { if (de_ctx->sig_list == NULL) { SCLogDebug("no signatures to order"); - return; + return 0; } + int retval = 0; SCLogDebug("ordering signatures in memory"); SCSigSignatureWrapper *sigw = NULL; SCSigSignatureWrapper *td_sigw_list = NULL; /* unified td list */ @@ -816,6 +817,12 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) Signature *sig = de_ctx->sig_list; while (sig != NULL) { sigw = SCSigAllocSignatureWrapper(sig); + if (sigw == NULL) { + SCLogError("failed to alloc signature wrapper for rule ordering"); + retval = -1; + goto cleanup; + } + /* Push signature wrapper onto a list, order doesn't matter here. */ if (sig->init_data->firewall_rule) { if (sig->type == SIG_TYPE_PKT) { @@ -851,6 +858,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) /* Recreate the sig list in order */ de_ctx->sig_list = NULL; +cleanup: /* firewall list for hook packet_filter */ for (sigw = fw_pf_sigw_list; sigw != NULL;) { SCLogDebug("post-sort packet_filter: sid %u", sigw->sig->id); @@ -901,6 +909,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) sigw = sigw->next; SCFree(sigw_to_free); } + return retval; } /** @@ -1059,7 +1068,7 @@ static int SCSigOrderingTest02(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1198,7 +1207,7 @@ static int SCSigOrderingTest03(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1313,7 +1322,7 @@ static int SCSigOrderingTest04(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1411,7 +1420,7 @@ static int SCSigOrderingTest05(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1500,7 +1509,7 @@ static int SCSigOrderingTest06(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1587,7 +1596,7 @@ static int SCSigOrderingTest07(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1687,7 +1696,7 @@ static int SCSigOrderingTest08(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1793,7 +1802,7 @@ static int SCSigOrderingTest09(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1897,7 +1906,7 @@ static int SCSigOrderingTest10(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1965,7 +1974,7 @@ static int SCSigOrderingTest11(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -2056,7 +2065,7 @@ static int SCSigOrderingTest13(void) FAIL_IF_NULL(sig); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowbitsCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); #ifdef DEBUG sig = de_ctx->sig_list; diff --git a/src/detect-engine-sigorder.h b/src/detect-engine-sigorder.h index d859846c62..358aa275c1 100644 --- a/src/detect-engine-sigorder.h +++ b/src/detect-engine-sigorder.h @@ -24,7 +24,7 @@ #ifndef SURICATA_DETECT_ENGINE_SIGORDER_H #define SURICATA_DETECT_ENGINE_SIGORDER_H -void SCSigOrderSignatures(DetectEngineCtx *); +int WARN_UNUSED SCSigOrderSignatures(DetectEngineCtx *); void SCSigRegisterSignatureOrderingFuncs(DetectEngineCtx *); void SCSigRegisterSignatureOrderingTests(void); void SCSigSignatureOrderingModuleCleanup(DetectEngineCtx *); diff --git a/src/detect-flowint.c b/src/detect-flowint.c index d83a9529f5..efc1054683 100644 --- a/src/detect-flowint.c +++ b/src/detect-flowint.c @@ -1131,7 +1131,7 @@ static int DetectFlowintTestPacket01Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1207,7 +1207,7 @@ static int DetectFlowintTestPacket02Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1280,7 +1280,7 @@ static int DetectFlowintTestPacket03Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 3) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); diff --git a/src/util-unittest-helper.c b/src/util-unittest-helper.c index 8aa92b226f..0d26588f19 100644 --- a/src/util-unittest-helper.c +++ b/src/util-unittest-helper.c @@ -738,7 +738,9 @@ int UTHMatchPackets(DetectEngineCtx *de_ctx, Packet **p, int num_packets) memset(&th_v, 0, sizeof(th_v)); StatsThreadInit(&th_v.stats); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + result = 0; + } SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);