fix null pointer dereferences found by malloc-error-check cocci - #15584
Closed
kenifor wants to merge 8 commits into
Closed
fix null pointer dereferences found by malloc-error-check cocci#15584kenifor wants to merge 8 commits into
kenifor wants to merge 8 commits into
Conversation
added 8 commits
June 9, 2026 11:17
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
SCStrdup result was stored and immediately used without checking for NULL, which would cause a NULL dereference if allocation fails.
The original pointer was overwritten with the SCRealloc result before checking for NULL, causing a memory leak if reallocation fails. Check the temporary pointer first before assigning.
Two SCStrdup calls that set ref->key had no NULL check. On allocation failure the pointer would be used immediately, causing a NULL dereference.
Two fixes: - Remove extra parentheses in existing NULL check: (*ext) -> *ext, which was causing the cocci script to miss the check as a false negative. - Simplify SCHSConfigInit to return SCCalloc() directly; the caller in detect-engine.c already checks the return value for NULL.
If SCCalloc fails, decoded is NULL and the subsequent SCBase64Decode call would dereference it. Return early on allocation failure.
Coccinelle uses OCaml Str, not PCRE. The '|' and '()' characters are literals in OCaml Str, so 'identifier func =~ "(SCMalloc|SCCalloc|...)"' never matched anything — making the entire script a no-op since its introduction. Replace all five patterns with OCaml Str alternation syntax 'A\|B'. Ticket: 8641
PacketAlertCreate is called from PacketInit on the packet allocation path. 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 UNITTESTS-only helpers in defrag.c accordingly: helpers returning Packet * use an explicit NULL check; the one returning int keeps the existing FAIL_IF style.
victorjulien
reviewed
Jun 9, 2026
| { | ||
| uint32_t decoded_len = SCBase64DecodeBufferSize((uint32_t)len); | ||
| uint8_t *decoded = SCCalloc(decoded_len, sizeof(uint8_t)); | ||
| if (decoded == NULL) |
Member
There was a problem hiding this comment.
should be abort on this? or use assert? I can't imagine we want alloc failures so silently continue here @catenacyber ?
Contributor
There was a problem hiding this comment.
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)
Member
There was a problem hiding this comment.
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?
Contributor
There was a problem hiding this comment.
I do not think it makes sense to check for that.
If we fail to allocate, this is more likely due to nallocinc
|
NOTE: This PR may contain new authors. |
victorjulien
approved these changes
Jun 10, 2026
Merged
Member
|
Merged in #15601, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #15560. Changes from previous iteration:
qa/cocci: added Ticket: 8641decode: commit message no longer references FatalError (not present in main)decode: defrag.c helpers — helpers returningPacket *use explicit NULL check; the one returningintkeeps FAIL_IF style