Skip to content

fix null pointer dereferences found by malloc-error-check cocci #15584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kenifor wants to merge 8 commits into from
Closed

fix null pointer dereferences found by malloc-error-check cocci #15584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
586aa66
util/log-redis: guard SCCalloc result for redis stream format
Jun 4, 2026
d8b5b43
detect/alert: guard SCStrdup result before use
Jun 4, 2026
f14b81c
detect/flowbits: check SCRealloc result before overwriting pointer
Jun 4, 2026
84b46f4
detect/reference: guard SCStrdup calls in DetectReferenceParse
Jun 4, 2026
7c9db62
util/mpm-hs: fix null check parentheses; simplify SCHSConfigInit
Jun 4, 2026
ca1f8a0
tests/fuzz: guard SCCalloc result in fuzz_decodebase64
Jun 4, 2026
72cca4c
qa/cocci: fix broken regex alternation in malloc-error-check
Jun 9, 2026
1dd6f8c
decode: propagate PacketAlertCreate failure instead of crashing
Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions qa/coccinelle/malloc-error-check.cocci
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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(...)
Expand All @@ -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";
@@

(
Expand All @@ -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(...)
Expand All @@ -33,7 +33,7 @@ x = SCRealloc(x, E1)
expression x, E1;
position malloced.p1;
statement S1, S2;
identifier func =~ "(SCMalloc|SCStrdup|SCCalloc|SCMallocAligned|SCRealloc)";
identifier func =~ "SCMalloc\|SCStrdup\|SCCalloc\|SCMallocAligned\|SCRealloc";
@@

x@p1 = func(...)
Expand Down
10 changes: 5 additions & 5 deletions src/decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ ExceptionPolicyStatsSetts flow_memcap_eps_stats = {
*/
PacketAlert *PacketAlertCreate(void)
{
PacketAlert *pa_array = SCCalloc(packet_alert_max, sizeof(PacketAlert));
DEBUG_VALIDATE_BUG_ON(pa_array == NULL);

return pa_array;
return SCCalloc(packet_alert_max, sizeof(PacketAlert));
}

void PacketAlertRecycle(PacketAlert *pa_array, uint16_t cnt)
Expand Down Expand Up @@ -267,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...");
Expand Down
17 changes: 13 additions & 4 deletions src/defrag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,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);
Expand Down Expand Up @@ -1222,7 +1225,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);
Expand Down Expand Up @@ -1277,7 +1280,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);
Expand Down Expand Up @@ -1347,7 +1353,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);
Expand Down
3 changes: 3 additions & 0 deletions src/detect-engine-alert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/detect-flowbits.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,11 +793,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;
Expand Down
6 changes: 6 additions & 0 deletions src/detect-reference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,19 @@ 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 {

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 {
Expand Down
6 changes: 5 additions & 1 deletion src/packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,16 @@ 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_id = 0;
p->livedev_dst_id = 0;
return true;
}

void PacketReleaseRefs(Packet *p)
Expand Down
2 changes: 1 addition & 1 deletion src/packet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/tests/fuzz/fuzz_decodebase64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ static void Base64FuzzTest(const uint8_t *src, size_t len)
{
uint32_t decoded_len = SCBase64DecodeBufferSize((uint32_t)len);
uint8_t *decoded = SCCalloc(decoded_len, sizeof(uint8_t));
if (decoded == NULL)

@victorjulien victorjulien Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be abort on this? or use assert? I can't imagine we want alloc failures so silently continue here @catenacyber ?

@catenacyber catenacyber Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think : no abort, no assert, yes we want alloc failures to continue silently
So we can work with nallocinc.c

Why would you want otherwise ?
(anyways, this is a detail)

@victorjulien victorjulien Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thinking was that if we fail to alloc, it's likely due to decoded_len calculation being way off. Would it make sense to have a check for that?

@catenacyber catenacyber Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it makes sense to check for that.

If we fail to allocate, this is more likely due to nallocinc

return;

for (uint8_t mode = SCBase64ModeRFC2045; mode <= SCBase64ModeStrict; mode++) {
(void)SCBase64Decode(src, len, mode, decoded);
Expand Down
3 changes: 3 additions & 0 deletions src/util-log-redis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/util-mpm-hs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -1178,8 +1178,7 @@ void SCHSPrintInfo(MpmCtx *mpm_ctx)

static MpmConfig *SCHSConfigInit(void)
{
MpmConfig *c = SCCalloc(1, sizeof(MpmConfig));
return c;
return SCCalloc(1, sizeof(MpmConfig));
}

static void SCHSConfigDeinit(MpmConfig **c)
Expand Down
Loading