From f96c2a58ff39f9c7f7f4bf95608bccc80aa7b36c Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 2 Jun 2026 21:25:15 +0200 Subject: [PATCH 01/17] output/filestore: refactor file descriptor handling To assist code analyzers. Gcc -fanalyzer got confused about it. Also test data pointer and length before calling fwrite and check the result better. Use a single atomic for the max open files check. --- src/output-filestore.c | 52 +++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/output-filestore.c b/src/output-filestore.c index 84a68aa4e8fc..a5e2a3fa3052 100644 --- a/src/output-filestore.c +++ b/src/output-filestore.c @@ -192,6 +192,9 @@ static void OutputFilestoreFinalizeFiles(ThreadVars *tv, const OutputFilestoreLo } } +/** + * \note `filestore_open_file_cnt` should only be used when FileGetMaxOpenFiles() is non-zero + */ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet *p, File *ff, void *tx, const uint64_t tx_id, const uint8_t *data, uint32_t data_len, uint8_t flags, uint8_t dir) @@ -201,6 +204,7 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet OutputFilestoreCtx *ctx = aft->ctx; char filename[PATH_MAX] = ""; int file_fd = -1; + bool close_file = false; SCLogDebug("ff %p, data %p, data_len %u", ff, data, data_len); @@ -218,18 +222,22 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet return -1; } - if (SC_ATOMIC_GET(filestore_open_file_cnt) < FileGetMaxOpenFiles()) { - SC_ATOMIC_ADD(filestore_open_file_cnt, 1); - ff->fd = file_fd; - } else { - if (FileGetMaxOpenFiles() > 0) { - StatsCounterIncr(&tv->stats, aft->counter_max_hits); - } + /* SC_ATOMIC_ADD returns value before the addition. */ + if (FileGetMaxOpenFiles() > 0 && + SC_ATOMIC_ADD(filestore_open_file_cnt, 1) >= FileGetMaxOpenFiles()) { + (void)SC_ATOMIC_SUB(filestore_open_file_cnt, 1); + StatsCounterIncr(&tv->stats, aft->counter_max_hits); ff->fd = -1; + close_file = true; /* not storing it, so need to close */ + } else if (FileGetMaxOpenFiles() == 0) { + close_file = true; /* max open files 0 means we immediately close */ + } else { + ff->fd = file_fd; } /* we can get called with NULL data when we need to close */ } else if (data != NULL) { - if (ff->fd == -1) { + file_fd = ff->fd; + if (file_fd == -1) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -242,14 +250,15 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet strerror(errno)); return -1; } - } else { - file_fd = ff->fd; + close_file = true; /* close temporary open */ } + } else if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { + file_fd = ff->fd; } - if (file_fd != -1) { + if (file_fd != -1 && data != NULL && data_len > 0) { ssize_t r = write(file_fd, (const void *)data, (size_t)data_len); - if (r == -1) { + if (r == -1 || (ssize_t)data_len != r) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -257,22 +266,23 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet StatsCounterIncr(&tv->stats, aft->fs_error_counter); WARN_ONCE(WOT_WRITE, "Filestore (v2) failed to write to %s: %s", filename, strerror(errno)); - if (ff->fd != -1) { + close_file = true; /* close in the error case */ + } + } + + /* close the open file if needed */ + if (file_fd != -1 && ((flags & OUTPUT_FILEDATA_FLAG_CLOSE) != 0 || close_file)) { + /* if it was stored in the ff, disconnect */ + if (ff->fd != -1) { + if (FileGetMaxOpenFiles()) { SC_ATOMIC_SUB(filestore_open_file_cnt, 1); } ff->fd = -1; } - if (ff->fd == -1) { - close(file_fd); - } + close(file_fd); } if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { - if (ff->fd != -1) { - close(ff->fd); - ff->fd = -1; - SC_ATOMIC_SUB(filestore_open_file_cnt, 1); - } OutputFilestoreFinalizeFiles(tv, aft, ctx, p, ff, tx, tx_id, dir); } From 50d970717b622405ff11e673ba82281e4ccf437f Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 10:13:39 +0200 Subject: [PATCH 02/17] frames: avoid possible undefined behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code analyzer flagged FrameCopy as a possible source of UB due to both pointers passed to memcpy being the same. app-layer-frames.c: In function ‘FrameCopy’: app-layer-frames.c:236:5: warning: overlapping buffers passed as arguments to ‘memcpy’ [-Wanalyzer-overlapping-buffers] 236 | memcpy(dst, src, sizeof(*dst)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘FramePrune’: events 1-8 │ │ 750 | static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) │ | ^~~~~~~~~~ │ | | │ | (1) entry to ‘FramePrune’ │...... │ 766 | for (uint16_t i = 0; i < frames->cnt; i++) { │ | ~~~~~~~~~~~~~~~ │ | | │ | (2) following ‘true’ branch... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 767 |│ if (i < FRAMES_STATIC_CNT) { │ |│ ~ │ |│ | │ |└──────────>(3) ...to here │ | (4) following ‘true’ branch (when ‘i <= 2’)... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 768 |│ Frame *frame = &frames->sframes[i]; │ |│ ~~~~~~~~~~~~~~~~~~ │ |│ | │ |└──────────────────────────────────────────>(5) ...to here │ 769 | FrameDebug("prune(s)", frames, frame); │ 770 | if (eof || FrameIsDone(frame, acked)) { │ | ~ │ | | │ | (6) following ‘false’ branch... ─>─┐ │ | │ │...... │ | │ │ |┌──────────────────────────────────────────────────┘ │ 779 |│ const uint64_t fle = FrameLeftEdge(stream, frame); │ |│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ |│ | │ |└────────────────────────────────────>(7) ...to here │ | (8) calling ‘FrameLeftEdge’ from ‘FramePrune’ │ └──> ‘FrameLeftEdge’: event 9 │ │ 257 | static inline uint64_t FrameLeftEdge(const TcpStream *stream, const Frame *frame) │ | ^~~~~~~~~~~~~ │ | | │ | (9) entry to ‘FrameLeftEdge’ │ ‘FrameLeftEdge’: event 10 │ │suricata-common.h:323:27: │ 323 | #define BUG_ON(x) assert(!(x)) │ | ^~~~~~ │ | | │ | (10) following ‘false’ branch (when ‘frame_offset <= app_progress’)... ─>─┐ │ | │ util-validate.h:95:36: note: in expansion of macro ‘BUG_ON’ │ 95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp)) │ | ^~~~~~ app-layer-frames.c:266:5: note: in expansion of macro ‘DEBUG_VALIDATE_BUG_ON’ │ 266 | DEBUG_VALIDATE_BUG_ON(frame_offset > app_progress); │ | ^~~~~~~~~~~~~~~~~~~~~ │ ‘FrameLeftEdge’: event 11 │ │ | │ │ |┌────────────────────────────────────────────────────────────────────────────────────────────────────┘ │ 269 |│ if (frame->len < 0) { │ |│ ~~~~~^~~~~ │ |│ | │ |└────────────>(11) ...to here │ <──────┘ │ ‘FramePrune’: events 12-13 │ │ 779 | const uint64_t fle = FrameLeftEdge(stream, frame); │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (12) returning to ‘FramePrune’ from ‘FrameLeftEdge’ │...... │ 783 | FrameCopy(nframe, frame); │ | ~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (13) calling ‘FrameCopy’ from ‘FramePrune’ │ └──> ‘FrameCopy’: events 14-15 │ │ 234 | static void FrameCopy(Frame *dst, Frame *src) │ | ^~~~~~~~~ │ | | │ | (14) entry to ‘FrameCopy’ │ 235 | { │ 236 | memcpy(dst, src, sizeof(*dst)); │ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (15) ⚠️ overlapping buffers passed as arguments to ‘memcpy’ │ In file included from suricata-common.h:129, from app-layer-frames.c:25: /usr/include/string.h:47:14: note: the behavior of ‘memcpy’ is undefined for overlapping buffers 47 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src, | ^~~~~~ --- src/app-layer-frames.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app-layer-frames.c b/src/app-layer-frames.c index 2d79fb463ab6..3962110414fa 100644 --- a/src/app-layer-frames.c +++ b/src/app-layer-frames.c @@ -233,6 +233,7 @@ static void FrameClean(Frame *frame) static void FrameCopy(Frame *dst, Frame *src) { + DEBUG_VALIDATE_BUG_ON(dst == src); memcpy(dst, src, sizeof(*dst)); } @@ -353,8 +354,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c #endif } else { Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -378,8 +379,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -780,8 +781,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) le = MIN(le, fle); SCLogDebug("le %" PRIu64 ", frame fle %" PRIu64, le, fle); Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; @@ -808,8 +809,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; From b3cd5c810ed46078ab513a8d00c34f1432f9b9b9 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:19:27 +0200 Subject: [PATCH 03/17] tm/queues: assist gcc -fanalyzer Work around TAILQ false positive. --- src/tm-queues.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tm-queues.c b/src/tm-queues.c index 81bbc3042b83..c36f97de4d4c 100644 --- a/src/tm-queues.c +++ b/src/tm-queues.c @@ -27,6 +27,7 @@ #include "threads.h" #include "tm-queues.h" #include "util-debug.h" +#include "util-validate.h" static TAILQ_HEAD(TmqList_, Tmq_) tmq_list = TAILQ_HEAD_INITIALIZER(tmq_list); @@ -83,6 +84,9 @@ void TmqResetQueues(void) while ((tmq = TAILQ_FIRST(&tmq_list))) { TAILQ_REMOVE(&tmq_list, tmq, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&tmq_list) == tmq); + if (tmq->name) { SCFree(tmq->name); } From a0e19a66587e296f0c5851001b3fe6bba67109e5 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:40:51 +0200 Subject: [PATCH 04/17] conf: assist gcc -fanalyzer Work around TAILQ false positive. --- src/conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf.c b/src/conf.c index 9c0c3e0ad4eb..49659266d80c 100644 --- a/src/conf.c +++ b/src/conf.c @@ -161,6 +161,8 @@ void SCConfNodeFree(SCConfNode *node) while ((tmp = TAILQ_FIRST(&node->head))) { TAILQ_REMOVE(&node->head, tmp, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&node->head) == tmp); SCConfNodeFree(tmp); } From 84ee18f222bd8489ab40d4443fab5752d209c218 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:48:04 +0200 Subject: [PATCH 05/17] decode/tcp: only set data ptr for valid option lengths --- src/decode-tcp.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/decode-tcp.c b/src/decode-tcp.c index a7d5ee4b1686..f2cad13d06bf 100644 --- a/src/decode-tcp.c +++ b/src/decode-tcp.c @@ -79,8 +79,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) } tcp_opts[tcp_opt_cnt].type = type; - tcp_opts[tcp_opt_cnt].len = olen; - tcp_opts[tcp_opt_cnt].data = (olen > 2) ? (pkt+2) : NULL; + tcp_opts[tcp_opt_cnt].len = olen; /* we are parsing the most commonly used opts to prevent * us from having to walk the opts list for these all the @@ -90,6 +89,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_WS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.wscale_set != 0) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -107,6 +107,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_MSS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.mss_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -119,6 +120,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_SACKOK_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (TCP_GET_SACKOK(p)) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -130,6 +132,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_TS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.ts_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -149,6 +152,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !((olen - 2) % 8 == 0)) { ENGINE_SET_EVENT(p, TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.sack_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -166,6 +170,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !(((olen - 2) & 0x1) == 0))) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.tfo_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -178,6 +183,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) case TCP_OPT_EXP2: SCLogDebug("TCP EXP option, len %u", olen); if (olen == 4 || olen == 12) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); uint16_t magic = DecodeTCPGetU16(tcp_opts[tcp_opt_cnt].data); if (magic == 0xf989) { if (p->l4.vars.tcp.tfo_set) { @@ -196,6 +202,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != 18) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.md5_option_present = true; } @@ -206,10 +213,18 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen < 4) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.ao_option_present = true; } break; + default: + if (olen > 2) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); + } else { + + tcp_opts[tcp_opt_cnt].data = NULL; + } } pkt += olen; From b02ec1402996b13feb0b2f3292160333a5d410f5 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:57:18 +0200 Subject: [PATCH 06/17] 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 ca9cda0ce0e7..038146a946b6 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -500,7 +500,10 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc } 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 9447b527393c..98012cf9d3f2 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 d859846c629e..358aa275c1a2 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 d83a9529f570..efc105468349 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 8aa92b226f65..0d26588f19a9 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); From b0301be8963a42d79d3490839cf193ed029c72dc Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 13:54:55 +0200 Subject: [PATCH 07/17] detect/flowvar: help gcc -fanalyzer Add debug validation statement to assert prev pointer is not NULL. --- src/detect-flowvar.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/detect-flowvar.c b/src/detect-flowvar.c index d5396742456f..e1e2397afa02 100644 --- a/src/detect-flowvar.c +++ b/src/detect-flowvar.c @@ -287,17 +287,16 @@ static int DetectFlowvarPostMatch( DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s, const SigMatchCtx *ctx) { - DetectVarList *fs, *prev; - const DetectFlowvarData *fd; - if (det_ctx->varlist == NULL) return 1; - fd = (const DetectFlowvarData *)ctx; + const DetectFlowvarData *fd = (const DetectFlowvarData *)ctx; + DetectVarList *prev = NULL; - prev = NULL; - fs = det_ctx->varlist; + DetectVarList *fs = det_ctx->varlist; while (fs != NULL) { + DetectVarList *next_fs = fs->next; + if (fd->idx == 0 || fd->idx == fs->idx) { SCLogDebug("adding to the flow %u:", fs->idx); //PrintRawDataFp(stdout, fs->buffer, fs->len); @@ -323,17 +322,16 @@ static int DetectFlowvarPostMatch( } if (fs == det_ctx->varlist) { - det_ctx->varlist = fs->next; SCFree(fs); - fs = det_ctx->varlist; + det_ctx->varlist = fs = next_fs; } else { - prev->next = fs->next; + DEBUG_VALIDATE_BUG_ON(prev == NULL); SCFree(fs); - fs = prev->next; + fs = prev->next = next_fs; } } else { prev = fs; - fs = fs->next; + fs = next_fs; } } return 1; From cf24071178e1aa184a4e296d3ee78112162e8e33 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 14:51:54 +0200 Subject: [PATCH 08/17] detect/ip_proto: clean up parsing function Helps address a gcc -fanalyzer warning. --- src/detect-ipproto.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/detect-ipproto.c b/src/detect-ipproto.c index 7fc05b15e826..22b5624f346b 100644 --- a/src/detect-ipproto.c +++ b/src/detect-ipproto.c @@ -83,13 +83,6 @@ void DetectIPProtoRegister(void) */ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) { - DetectIPProtoData *data = NULL; - char *args[2] = { NULL, NULL }; - int res = 0; - size_t pcre2_len; - int i; - const char *str_ptr; - /* Execute the regex and populate args with captures. */ pcre2_match_data *match = NULL; int ret = DetectParsePcreExec(&parse_regex, &match, optstr, 0, 0); @@ -97,11 +90,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) SCLogError("pcre_exec parse error, ret" "%" PRId32 ", string %s", ret, optstr); - goto error; + if (match) { + pcre2_match_data_free(match); + } + return NULL; } - for (i = 0; i < (ret - 1); i++) { - res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); + char *args[2] = { NULL, NULL }; + DetectIPProtoData *data = NULL; + + for (int i = 0; i < 2; i++) { + const char *str_ptr = NULL; + size_t pcre2_len = 0; + int res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); if (res < 0) { SCLogError("pcre2_substring_get_bynumber failed"); goto error; @@ -110,7 +111,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) } /* Initialize the data */ - data = SCMalloc(sizeof(DetectIPProtoData)); + data = SCCalloc(1, sizeof(DetectIPProtoData)); if (unlikely(data == NULL)) goto error; data->op = DETECT_IPPROTO_OP_EQ; @@ -125,19 +126,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (!isdigit((unsigned char)*(args[1]))) { uint8_t proto; if (!SCGetProtoByName(args[1], &proto)) { - SCLogError("Unknown protocol name: \"%s\"", str_ptr); + SCLogError("Unknown protocol name: \"%s\"", args[1]); goto error; } data->proto = proto; } else { if (StringParseUint8(&data->proto, 10, 0, args[1]) <= 0) { - SCLogError("Malformed protocol number: %s", str_ptr); + SCLogError("Malformed protocol number: %s", args[1]); goto error; } } - for (i = 0; i < (ret - 1); i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } @@ -149,7 +150,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (match) { pcre2_match_data_free(match); } - for (i = 0; i < (ret - 1) && i < 2; i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } From af62b2d5d535fd852c48c0700f48dae626b0a315 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:05:46 +0200 Subject: [PATCH 09/17] log-pcap: address gcc analyzer warnings Reopen file descriptor for lz4 with the init function. This helps code analyzers understand the handle it's leaked. Improve flow of profiling dumps to avoid analyzer confusion around the file descriptor. Suppress TAILQ related warnings. --- src/log-pcap.c | 66 ++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/log-pcap.c b/src/log-pcap.c index 077fd9f6a481..7d6f3ccd018d 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -129,11 +129,11 @@ typedef struct PcapLogCompressionData_ { #ifdef HAVE_LIBLZ4 LZ4F_compressionContext_t lz4f_context; LZ4F_preferences_t lz4f_prefs; + FILE *pcap_buf_wrapper; #endif /* HAVE_LIBLZ4 */ FILE *file; uint8_t *pcap_buf; uint64_t pcap_buf_size; - FILE *pcap_buf_wrapper; uint64_t bytes_in_block; } PcapLogCompressionData; @@ -277,15 +277,7 @@ static int PcapLogCloseFile(ThreadVars *t, PcapLogData *pl) #ifdef HAVE_LIBLZ4 PcapLogCompressionData *comp = &pl->compression; if (comp->format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { - /* pcap_dump_close() has closed its output ``file'', - * so we need to call fmemopen again. */ - - comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, - comp->pcap_buf_size, "w"); - if (comp->pcap_buf_wrapper == NULL) { - SCLogError("SCFmemopen failed: %s", strerror(errno)); - return TM_ECODE_FAILED; - } + comp->pcap_buf_wrapper = NULL; } #endif /* HAVE_LIBLZ4 */ } @@ -367,6 +359,7 @@ static int PcapLogRotateFile(ThreadVars *t, PcapLogData *pl) } TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); pl->file_cnt--; } @@ -442,6 +435,12 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p) pl->fopen_err = 0; } + comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, comp->pcap_buf_size, "w"); + if (comp->pcap_buf_wrapper == NULL) { + fclose(comp->file); + comp->file = NULL; + return TM_ECODE_FAILED; + } if ((pl->pcap_dumper = pcap_dump_fopen(pl->pcap_dead_handle, comp->pcap_buf_wrapper)) == NULL) { if (!pl->pcap_open_err) { @@ -1015,6 +1014,7 @@ static TmEcode PcapLogInitRingBuffer(PcapLogData *pl) PcapFileName *pf = TAILQ_FIRST(&pl->pcap_file_list); while (pf != NULL && pl->file_cnt > pl->max_files) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); SCLogDebug("Removing PCAP file %s", pf->filename); if (remove(pf->filename) != 0) { @@ -1166,6 +1166,7 @@ static void PcapLogDataFree(PcapLogData *pl) PcapFileName *pf; while ((pf = TAILQ_FIRST(&pl->pcap_file_list)) != NULL) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); } if (pl == g_pcap_data) { @@ -1191,7 +1192,8 @@ static void PcapLogDataFree(PcapLogData *pl) #ifdef HAVE_LIBLZ4 if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { SCFree(pl->compression.buffer); - fclose(pl->compression.pcap_buf_wrapper); + if (pl->compression.pcap_buf_wrapper) + fclose(pl->compression.pcap_buf_wrapper); SCFree(pl->compression.pcap_buf); LZ4F_errorCode_t errcode = LZ4F_freeCompressionContext(pl->compression.lz4f_context); @@ -1217,7 +1219,7 @@ static TmEcode PcapLogDataDeinit(ThreadVars *t, void *thread_data) PcapLogData *pl = td->pcap_log; if (pl->pcap_dumper != NULL) { - if (PcapLogCloseFile(t,pl) < 0) { + if (PcapLogCloseFile(t, pl) != TM_ECODE_OK) { SCLogDebug("PcapLogCloseFile failed"); } } @@ -1482,7 +1484,9 @@ static OutputInitResult PcapLogInitCtx(SCConfNode *conf) comp->file = NULL; comp->pcap_buf = NULL; comp->pcap_buf_size = 0; +#ifdef HAVE_LIBLZ4 comp->pcap_buf_wrapper = NULL; +#endif } else if (strcmp(compression_str, "lz4") == 0) { #ifdef HAVE_LIBLZ4 pl->compression.format = PCAP_LOG_COMPRESSION_FORMAT_LZ4; @@ -1849,7 +1853,7 @@ static void FormatNumber(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fb", (float)num/1000000000UL); } -static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) +static void ProfileReportPair(FILE *fp, const char *name, const PcapLogProfileData *p) { char ticks_str[32] = "n/a"; char cnt_str[32] = "n/a"; @@ -1863,7 +1867,7 @@ static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) fprintf(fp, "%-28s %-10s %-10s %-10s\n", name, cnt_str, avg_str, ticks_str); } -static void ProfileReport(FILE *fp, PcapLogData *pl) +static void ProfileReport(FILE *fp, const PcapLogData *pl) { ProfileReportPair(fp, "open", &pl->profile_open); ProfileReportPair(fp, "close", &pl->profile_close); @@ -1886,23 +1890,8 @@ static void FormatBytes(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fGiB", (float)num/1000000000UL); } -static void PcapLogProfilingDump(PcapLogData *pl) +static void DoDump(const PcapLogData *pl, FILE *fp) { - FILE *fp = NULL; - - if (profiling_pcaplog_enabled == 0) - return; - - if (profiling_pcaplog_output_to_file == 1) { - fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); - if (fp == NULL) { - SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); - return; - } - } else { - fp = stdout; - } - /* counters */ fprintf(fp, "\n\nOperation Cnt Avg ticks Total ticks\n"); fprintf(fp, "---------------------------- ---------- ---------- -----------\n"); @@ -1942,9 +1931,24 @@ static void PcapLogProfilingDump(PcapLogData *pl) if (ticks_per_gib > 0) FormatNumber(ticks_per_gib, ticks_per_gib_str, sizeof(ticks_per_gib_str)); fprintf(fp, " Ticks per GiB: %s\n", ticks_per_gib_str); +} - if (fp != stdout) +static void PcapLogProfilingDump(PcapLogData *pl) +{ + if (profiling_pcaplog_enabled == 0) + return; + + if (profiling_pcaplog_output_to_file == 1) { + FILE *fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); + if (fp == NULL) { + SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); + return; + } + DoDump(pl, fp); fclose(fp); + } else { + DoDump(pl, stdout); + } } void PcapLogProfileSetup(void) From 7d5f4c26d578b9d3e977708e00a839a73344906b Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:06:03 +0200 Subject: [PATCH 10/17] output: suppress gcc analyzer warnings By teaching about TAILQ. --- src/output.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/output.c b/src/output.c index e9f76f4e79d9..cdecfe519fdb 100644 --- a/src/output.c +++ b/src/output.c @@ -656,6 +656,7 @@ void OutputDeregisterAll(void) while ((module = TAILQ_FIRST(&output_modules))) { TAILQ_REMOVE(&output_modules, module, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&output_modules) == module); SCFree(module); } SCFree(simple_json_applayer_loggers); @@ -898,6 +899,7 @@ void OutputClearActiveLoggers(void) RootLogger *logger; while ((logger = TAILQ_FIRST(&active_loggers)) != NULL) { TAILQ_REMOVE(&active_loggers, logger, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&active_loggers) == logger); SCFree(logger); } } From 1f268dae0eb603c27b1d2d641c47d4c55cd3cb14 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:06:23 +0200 Subject: [PATCH 11/17] affinity: gcc analyzer warnings The double strchr confused gcc -fanalyzer. --- src/util-affinity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util-affinity.c b/src/util-affinity.c index cba5aec1d60a..9edd601c4f27 100644 --- a/src/util-affinity.c +++ b/src/util-affinity.c @@ -229,6 +229,7 @@ int BuildCpusetWithCallback( { SCConfNode *lnode; TAILQ_FOREACH(lnode, &node->head, next) { + char *sep = NULL; uint32_t i; uint32_t a, b; uint32_t stop = 0; @@ -240,8 +241,7 @@ int BuildCpusetWithCallback( a = 0; b = max; stop = 1; - } else if (strchr(lnode->val, '-') != NULL) { - char *sep = strchr(lnode->val, '-'); + } else if ((sep = strchr(lnode->val, '-')) != NULL) { if (StringParseUint32(&a, 10, sep - lnode->val, lnode->val) <= 0) { SCLogError("%s: invalid cpu range (start invalid): \"%s\"", name, lnode->val); return -1; From 28bd0a66e467b9ec81113eb06c74e63a969a268e Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:33:26 +0200 Subject: [PATCH 12/17] spm/bm: match suff array to pattern size Avoids gcc -fanalyzer warning about out of bounds write to the array. --- src/util-spm-bm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util-spm-bm.c b/src/util-spm-bm.c index bfdb28600aa3..850662e42fa0 100644 --- a/src/util-spm-bm.c +++ b/src/util-spm-bm.c @@ -184,7 +184,7 @@ static void BoyerMooreSuffixes(const uint8_t *x, uint16_t m, uint16_t *suff) static int PreBmGs(const uint8_t *x, uint16_t m, uint16_t *bmGs) { int32_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixes(x, m, suff); @@ -260,7 +260,7 @@ static void BoyerMooreSuffixesNocase(const uint8_t *x, uint16_t m, static void PreBmGsNocase(const uint8_t *x, uint16_t m, uint16_t *bmGs) { uint16_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixesNocase(x, m, suff); From 729325dbfd16969e819c144a9e88c40e1355a11b Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:34:51 +0200 Subject: [PATCH 13/17] util/var-name: help gcc analyzer Help it understand TAILQ. --- src/util-var-name.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util-var-name.c b/src/util-var-name.c index a24c9f4874de..f62675950f09 100644 --- a/src/util-var-name.c +++ b/src/util-var-name.c @@ -127,6 +127,7 @@ void VarNameStoreDestroy(void) while ((s = TAILQ_FIRST(&free_list))) { TAILQ_REMOVE(&free_list, s, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&free_list) == s); HashListTableFree(s->names); HashListTableFree(s->ids); SCFree(s); From 37339c0de1bdffea1a9b900bd723f7f98714b218 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 13:03:36 +0200 Subject: [PATCH 14/17] mpm/hs: remove useless pointer check Pointer can't be NULL, so don't check it. Helps gcc analyzer as well. --- src/util-mpm-hs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util-mpm-hs.c b/src/util-mpm-hs.c index 3b77a4de44c5..a8b1e2d0ac22 100644 --- a/src/util-mpm-hs.c +++ b/src/util-mpm-hs.c @@ -775,7 +775,8 @@ int SCHSPreparePatterns(MpmConfig *mpm_conf, MpmCtx *mpm_ctx) } const char *cache_path = pd->no_cache || !mpm_conf ? NULL : mpm_conf->cache_dir_path; - if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0 && pd != NULL) { + if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0) { + DEBUG_VALIDATE_BUG_ON(pd == NULL); cd = NULL; ctx->pattern_db = pd; if (PatternDatabaseGetSize(pd, &ctx->hs_db_size) != 0) { From 55bc96a00a57cce2cc7767e45df31a03c891f58b Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 15:15:16 +0200 Subject: [PATCH 15/17] nfq: suppress gcc analyzer warnings --- src/source-nfq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/source-nfq.c b/src/source-nfq.c index 7f75cd171c1b..e664503b35f2 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -567,6 +567,7 @@ static int NFQCallBack(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg, if (ret == -1) { #ifdef COUNTERS NFQQueueVars *q = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(q == NULL); q->errs++; q->pkts++; q->bytes += GET_PKT_LEN(p); @@ -976,6 +977,7 @@ static void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv) { int ret; int flag = NFQVerdictCacheLen(t) ? MSG_DONTWAIT : 0; + DEBUG_VALIDATE_BUG_ON(t == NULL); int rv = recv(t->fd, tv->data, tv->datalen, flag); if (rv < 0) { @@ -1049,6 +1051,7 @@ void ReceiveNFQThreadExitStats(ThreadVars *tv, void *data) { NFQThreadVars *ntv = (NFQThreadVars *)data; NFQQueueVars *nq = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(nq == NULL); #ifdef COUNTERS SCLogNotice("(%s) Treated: Pkts %" PRIu32 ", Bytes %" PRIu64 ", Errors %" PRIu32 "", tv->name, nq->pkts, nq->bytes, nq->errs); From 1e880ce00d7e08772aa1a87782db8741c814e2c6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 15:16:02 +0200 Subject: [PATCH 16/17] github-ci: add gcc analyzer build Make sure to not run against lua rust crate build, as it's not clean. --- .github/workflows/scan-build.yml | 60 ++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/.github/workflows/scan-build.yml b/.github/workflows/scan-build.yml index 49691d2d1934..990f0fa97a9c 100644 --- a/.github/workflows/scan-build.yml +++ b/.github/workflows/scan-build.yml @@ -165,3 +165,63 @@ jobs: name: scan-build-results path: scan-build-report/ retention-days: 5 + gcc-analyzer: + name: GCC analyzer + runs-on: ubuntu-latest + container: ubuntu:26.04 + steps: + - name: Cache scan-build + uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb + with: + path: ~/.cargo + key: scan-build + + - name: Install system packages + run: | + apt update + apt -y install \ + libpcre2-dev \ + build-essential \ + autoconf \ + automake \ + cargo \ + cbindgen \ + dpdk-dev \ + gcc-16 \ + git \ + libtool \ + libpcap-dev \ + libnet1-dev \ + libyaml-0-2 \ + libyaml-dev \ + libcap-ng-dev \ + libcap-ng0 \ + libmagic-dev \ + libnetfilter-log-dev \ + libnetfilter-queue-dev \ + libnetfilter-queue1 \ + libnfnetlink-dev \ + libnfnetlink0 \ + libnuma-dev \ + libhiredis-dev \ + libhyperscan-dev \ + libjansson-dev \ + libevent-dev \ + libevent-pthreads-2.1-7 \ + liblz4-dev \ + make \ + python3-yaml \ + rustc \ + software-properties-common \ + zlib1g \ + zlib1g-dev + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + - run: git config --global --add safe.directory /__w/suricata/suricata + - run: ./scripts/bundle.sh + - run: ./autogen.sh + - run: ./configure --enable-warnings --enable-dpdk --enable-nfqueue --enable-nflog --enable-debug-validation + env: + CC: gcc-16 + CFLAGS: "-fanalyzer -Werror" + SURICATA_LUA_SYS_CFLAGS: "" + - run: make From 11982039f7021db8bff9a29ac03939021031abba Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 11 Jun 2026 11:15:14 +0200 Subject: [PATCH 17/17] ci: check-doc-rules copies data(sets) files into tmpdir Allows to use datasets load --- scripts/check-doc-rules.py | 3 +++ scripts/docrules/data/blocked-domains.lst | 1 + 2 files changed, 4 insertions(+) create mode 100644 scripts/docrules/data/blocked-domains.lst diff --git a/scripts/check-doc-rules.py b/scripts/check-doc-rules.py index e84338387de0..2785039c539f 100644 --- a/scripts/check-doc-rules.py +++ b/scripts/check-doc-rules.py @@ -111,9 +111,11 @@ def check_rule_with_suricata( rule: str, suricata_bin: Path, suricata_yaml: Path, + data_dir: Path, ) -> Tuple[bool, str]: with tempfile.TemporaryDirectory(prefix="doc-rule-check-") as tmpdir: rule_file = Path(tmpdir) / "rule.rules" + shutil.copytree(data_dir, tmpdir, dirs_exist_ok=True) rule_file.write_text(rule + "\n", encoding="utf-8") cmd = [ @@ -185,6 +187,7 @@ def main() -> int: rule, suricata_bin, suricata_yaml, + repo_root / "scripts" / "docrules" / "data", ) if not is_valid: print( diff --git a/scripts/docrules/data/blocked-domains.lst b/scripts/docrules/data/blocked-domains.lst new file mode 100644 index 000000000000..26a0127725a8 --- /dev/null +++ b/scripts/docrules/data/blocked-domains.lst @@ -0,0 +1 @@ +LmV2aWwuY29tCg==