From 8abe6747716808a0ae8304c2507b675d11420f08 Mon Sep 17 00:00:00 2001 From: Denis Balashov Date: Thu, 14 May 2026 17:20:15 +0300 Subject: [PATCH 1/5] redis: guard stream format allocation When Redis output is configured in stream/xadd mode with a positive stream-maxlen, SCConfLogOpenRedis() allocates redis_setup.stream_format and immediately passes it to snprintf(). If SCCalloc() fails, snprintf() receives a NULL destination pointer and the process can crash during Redis output initialization. Handle this unrecoverable setup failure with FatalError(), matching the surrounding Redis initialization error handling. Ticket: 8588 --- src/util-log-redis.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util-log-redis.c b/src/util-log-redis.c index f0755e7473eb..adc6b23f7e59 100644 --- a/src/util-log-redis.c +++ b/src/util-log-redis.c @@ -673,6 +673,9 @@ int SCConfLogOpenRedis(SCConfNode *redis_node, void *lf_ctx) format string, whose length is limited by the length of the maxlen integer formatted as a string */ log_ctx->redis_setup.stream_format = SCCalloc(100, sizeof(char)); + if (unlikely(log_ctx->redis_setup.stream_format == NULL)) { + FatalError("Unable to allocate redis stream format"); + } snprintf(log_ctx->redis_setup.stream_format, 100, redis_stream_format_maxlen_tmpl, "%s", "%s", exact ? '=' : '~', maxlen, "%s"); log_ctx->redis_setup.format = log_ctx->redis_setup.stream_format; From 4111d98a6c18146d262143d19a98b1b320426105 Mon Sep 17 00:00:00 2001 From: Denis Balashov Date: Thu, 28 May 2026 17:11:15 +0300 Subject: [PATCH 2/5] qa: fix cocci malloc-error-check to catch use-before-null-check via function args The @istested rule's '... when != x' only excluded reassignments of x, not statements that passed x as a function argument. Add 'when != callee(..., x, ...)' so that any use of the allocated pointer as a function argument before a NULL check is no longer treated as tested and is correctly flagged. --- qa/coccinelle/malloc-error-check.cocci | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qa/coccinelle/malloc-error-check.cocci b/qa/coccinelle/malloc-error-check.cocci index ce4d0cc47f96..749297725bd5 100644 --- a/qa/coccinelle/malloc-error-check.cocci +++ b/qa/coccinelle/malloc-error-check.cocci @@ -34,10 +34,12 @@ expression x, E1; position malloced.p1; statement S1, S2; identifier func =~ "(SCMalloc|SCStrdup|SCCalloc|SCMallocAligned|SCRealloc)"; +identifier callee; @@ x@p1 = func(...) ... when != x + when != callee(..., x, ...) ( if (unlikely(x == NULL)) S1 | From 3ca8259f6043c008c3479da5be7c4b649800e365 Mon Sep 17 00:00:00 2001 From: Denis Balashov Date: Tue, 2 Jun 2026 16:19:36 +0300 Subject: [PATCH 3/5] qa/cocci: fix broken regex alternation in malloc-error-check The identifier regex used PCRE-style '(A|B)' syntax, but Coccinelle uses OCaml Str where '|' is a literal character. Replace with 'A\|B' so the pattern correctly matches all SC*alloc functions. --- qa/coccinelle/malloc-error-check.cocci | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/qa/coccinelle/malloc-error-check.cocci b/qa/coccinelle/malloc-error-check.cocci index 749297725bd5..d7d1bae8cf5b 100644 --- a/qa/coccinelle/malloc-error-check.cocci +++ b/qa/coccinelle/malloc-error-check.cocci @@ -1,7 +1,7 @@ @malloced@ expression x; position p1; -identifier func =~ "(SCMalloc|SCStrdup|SCCalloc|SCMallocAligned|SCRealloc)"; +identifier func =~ "SCMalloc\|SCStrdup\|SCCalloc\|SCMallocAligned\|SCRealloc"; @@ x@p1 = func(...) @@ -10,7 +10,7 @@ x@p1 = func(...) expression x, E; statement S; position malloced.p1; -identifier func =~ "(SCMalloc|SCStrdup|SCCalloc|SCMallocAligned|SCRealloc)"; +identifier func =~ "SCMalloc\|SCStrdup\|SCCalloc\|SCMallocAligned\|SCRealloc"; @@ ( @@ -22,7 +22,7 @@ if (E && (x@p1 = func(...)) == NULL) S @realloc exists@ position malloced.p1; expression x, E1; -identifier func =~ "(SCMalloc|SCCalloc|SCMallocAligned)"; +identifier func =~ "SCMalloc\|SCCalloc\|SCMallocAligned"; @@ x@p1 = func(...) @@ -33,13 +33,11 @@ x = SCRealloc(x, E1) expression x, E1; position malloced.p1; statement S1, S2; -identifier func =~ "(SCMalloc|SCStrdup|SCCalloc|SCMallocAligned|SCRealloc)"; -identifier callee; +identifier func =~ "SCMalloc\|SCStrdup\|SCCalloc\|SCMallocAligned\|SCRealloc"; @@ x@p1 = func(...) ... when != x - when != callee(..., x, ...) ( if (unlikely(x == NULL)) S1 | From 207594e2b742518edaaedb84c2876fea97305e61 Mon Sep 17 00:00:00 2001 From: Denis Balashov Date: Wed, 3 Jun 2026 11:27:23 +0300 Subject: [PATCH 4/5] fix null pointer dereferences found by malloc-error-check cocci - decode: replace DEBUG_VALIDATE_BUG_ON with FatalError in PacketAlertCreate - detect-engine-alert: guard SCStrdup result before use - detect-flowbits: check SCRealloc result before overwriting original pointer - detect-reference: guard SCStrdup results in DetectReferenceParse - util-mpm-hs: fix false cocci negative (remove extra parens), add FatalError in SCHSConfigInit --- src/decode.c | 4 +++- src/detect-engine-alert.c | 3 +++ src/detect-flowbits.c | 4 ++-- src/detect-reference.c | 6 ++++++ src/util-mpm-hs.c | 5 ++++- 5 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/decode.c b/src/decode.c index 6dd6701bfb0c..69a0163059fa 100644 --- a/src/decode.c +++ b/src/decode.c @@ -144,7 +144,9 @@ ExceptionPolicyStatsSetts flow_memcap_eps_stats = { PacketAlert *PacketAlertCreate(void) { PacketAlert *pa_array = SCCalloc(packet_alert_max, sizeof(PacketAlert)); - DEBUG_VALIDATE_BUG_ON(pa_array == NULL); + if (unlikely(pa_array == NULL)) { + FatalError("Failed to allocate packet alert array"); + } return pa_array; } diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c index 1dcc78f4c86b..335c546306a2 100644 --- a/src/detect-engine-alert.c +++ b/src/detect-engine-alert.c @@ -337,6 +337,9 @@ static inline int PacketAlertSetContext( } } current_json->json_string = SCStrdup(det_ctx->json_content[i].json_content); + if (current_json->json_string == NULL) { + return -1; + } SCLogDebug("json content %u, value '%s' (%p)", (unsigned int)i, current_json->json_string, s); } diff --git a/src/detect-flowbits.c b/src/detect-flowbits.c index 438a2915f216..f443b510d541 100644 --- a/src/detect-flowbits.c +++ b/src/detect-flowbits.c @@ -791,11 +791,11 @@ int DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx) uint32_t new_fb_array_size = s->init_data->rule_state_flowbits_ids_size + 1; void *tmp_fb_ptr = SCRealloc(s->init_data->rule_state_flowbits_ids_array, new_fb_array_size * sizeof(uint32_t)); - s->init_data->rule_state_flowbits_ids_array = tmp_fb_ptr; - if (s->init_data->rule_state_flowbits_ids_array == NULL) { + if (tmp_fb_ptr == NULL) { SCLogError("Failed to reallocate memory for rule_state_variable_idx"); goto error; } + s->init_data->rule_state_flowbits_ids_array = tmp_fb_ptr; SCLogDebug( "realloc'ed array for flowbits ids, new size is %u", new_fb_array_size); s->init_data->rule_state_dependant_sids_size = new_array_size; diff --git a/src/detect-reference.c b/src/detect-reference.c index 2981f3e5b68c..700d149c8c21 100644 --- a/src/detect-reference.c +++ b/src/detect-reference.c @@ -153,6 +153,9 @@ static DetectReference *DetectReferenceParse(const char *rawstr, DetectEngineCtx if (strlen(scheme)) { SCLogConfig("scheme value %s overrides key %s", scheme, key); ref->key = SCStrdup(scheme); + if (ref->key == NULL) { + goto error; + } /* already bound checked to be REFERENCE_SYSTEM_NAME_MAX or less */ ref->key_len = (uint16_t)strlen(scheme); } else { @@ -160,6 +163,9 @@ static DetectReference *DetectReferenceParse(const char *rawstr, DetectEngineCtx SCRConfReference *lookup_ref_conf = SCRConfGetReference(key, de_ctx); if (lookup_ref_conf != NULL) { ref->key = SCStrdup(lookup_ref_conf->url); + if (ref->key == NULL) { + goto error; + } /* already bound checked to be REFERENCE_SYSTEM_NAME_MAX or less */ ref->key_len = (uint16_t)strlen(ref->key); } else { diff --git a/src/util-mpm-hs.c b/src/util-mpm-hs.c index 2d4daba19992..2a0b7d9675d0 100644 --- a/src/util-mpm-hs.c +++ b/src/util-mpm-hs.c @@ -641,7 +641,7 @@ static int CompileDataExtensionsInit(hs_expr_ext_t **ext, const SCHSPattern *p) { if (p->flags & (MPM_PATTERN_FLAG_OFFSET | MPM_PATTERN_FLAG_DEPTH)) { *ext = SCCalloc(1, sizeof(hs_expr_ext_t)); - if ((*ext) == NULL) { + if (*ext == NULL) { return -1; } if (p->flags & MPM_PATTERN_FLAG_OFFSET) { @@ -1179,6 +1179,9 @@ void SCHSPrintInfo(MpmCtx *mpm_ctx) static MpmConfig *SCHSConfigInit(void) { MpmConfig *c = SCCalloc(1, sizeof(MpmConfig)); + if (unlikely(c == NULL)) { + FatalError("Failed to allocate MpmConfig"); + } return c; } From 4ae1d799585926edcce08d9f50a3272e25a39d19 Mon Sep 17 00:00:00 2001 From: Denis Balashov Date: Thu, 4 Jun 2026 11:24:07 +0300 Subject: [PATCH 5/5] decode: propagate PacketAlertCreate failure instead of FatalError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PacketAlertCreate is called from PacketInit which is on the packet allocation path. Using FatalError there is inappropriate — on allocation failure the packet should be dropped, not the process terminated. Make PacketInit return bool and propagate the NULL result from PacketAlertCreate up through PacketGetFromAlloc, which already returns NULL to signal allocation failure to its callers. Update the four UNITTESTS-only helpers in defrag.c accordingly. --- src/decode.c | 12 +++++------- src/defrag.c | 17 +++++++++++++---- src/packet.c | 6 +++++- src/packet.h | 2 +- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/decode.c b/src/decode.c index 69a0163059fa..784f591598bc 100644 --- a/src/decode.c +++ b/src/decode.c @@ -143,12 +143,7 @@ ExceptionPolicyStatsSetts flow_memcap_eps_stats = { */ PacketAlert *PacketAlertCreate(void) { - PacketAlert *pa_array = SCCalloc(packet_alert_max, sizeof(PacketAlert)); - if (unlikely(pa_array == NULL)) { - FatalError("Failed to allocate packet alert array"); - } - - return pa_array; + return SCCalloc(packet_alert_max, sizeof(PacketAlert)); } void PacketAlertRecycle(PacketAlert *pa_array, uint16_t cnt) @@ -269,7 +264,10 @@ Packet *PacketGetFromAlloc(void) if (unlikely(p == NULL)) { return NULL; } - PacketInit(p); + if (!PacketInit(p)) { + SCFree(p); + return NULL; + } p->ReleasePacket = PacketFree; SCLogDebug("allocated a new packet only using alloc..."); diff --git a/src/defrag.c b/src/defrag.c index 04294d7e726f..207a55a538f9 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -1160,7 +1160,10 @@ static Packet *BuildIpv4TestPacket( if (unlikely(p == NULL)) return NULL; - PacketInit(p); + if (!PacketInit(p)) { + SCFree(p); + return NULL; + } struct timeval tval; gettimeofday(&tval, NULL); @@ -1224,7 +1227,7 @@ static int BuildIpv4TestPacketWithContent(Packet **packet, uint8_t proto, uint16 p = SCCalloc(1, sizeof(*p) + default_packet_size); FAIL_IF_NULL(p); - PacketInit(p); + FAIL_IF(!PacketInit(p)); struct timeval tval; gettimeofday(&tval, NULL); @@ -1279,7 +1282,10 @@ static Packet *BuildIpv6TestPacket( if (unlikely(p == NULL)) return NULL; - PacketInit(p); + if (!PacketInit(p)) { + SCFree(p); + return NULL; + } struct timeval tval; gettimeofday(&tval, NULL); @@ -1349,7 +1355,10 @@ static Packet *BuildIpv6TestPacketWithContent( if (unlikely(p == NULL)) return NULL; - PacketInit(p); + if (!PacketInit(p)) { + SCFree(p); + return NULL; + } struct timeval tval; gettimeofday(&tval, NULL); diff --git a/src/packet.c b/src/packet.c index 67df4d6f1b2f..7fc4f7f37f62 100644 --- a/src/packet.c +++ b/src/packet.c @@ -70,11 +70,15 @@ uint8_t PacketGetAction(const Packet *p) /** * \brief Initialize a packet structure for use. */ -void PacketInit(Packet *p) +bool PacketInit(Packet *p) { SCSpinInit(&p->persistent.tunnel_lock, 0); p->alerts.alerts = PacketAlertCreate(); + if (unlikely(p->alerts.alerts == NULL)) { + return false; + } p->livedev = NULL; + return true; } void PacketReleaseRefs(Packet *p) diff --git a/src/packet.h b/src/packet.h index 5ad43c3ddfcc..044068ad17fc 100644 --- a/src/packet.h +++ b/src/packet.h @@ -32,7 +32,7 @@ static inline uint8_t PacketTestAction(const Packet *p, const uint8_t a) } #endif -void PacketInit(Packet *p); +bool PacketInit(Packet *p); void PacketReleaseRefs(Packet *p); void PacketReinit(Packet *p); void PacketRecycle(Packet *p);