fix: unchecked allocations, log newline overflow, and leaked result sets - #576
Closed
somethingwithproof wants to merge 3 commits into
Closed
fix: unchecked allocations, log newline overflow, and leaked result sets#576somethingwithproof wants to merge 3 commits into
somethingwithproof wants to merge 3 commits into
Conversation
getsetting(), getpsetting(), getglobalvariable() and the Cacti version read each returned from the NULL-row branch without freeing, while every other exit in those functions frees. Closes Cacti#566 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The strncat() calls are bounded to fill the buffer exactly, after which strcat() wrote the newline at LOGSIZE-1 and its terminator one past the end. The newline is now only appended when a byte is free, and replaces the last character otherwise. Closes Cacti#565 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Member
Author
|
Superseded by #578, which carries the same Closing to keep one review rather than two for the same fixes. #564, #565 and #566 are tracked on #578. |
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
1 participant
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.
Three defects, one commit each.
Closes #564 — unchecked
calloc()in spine.c.php_processes,debug_devices,db_pool_localanddb_pool_remoteare dereferenced on the statement after allocation. #542 gave the two in poller.c adie(); these were missed. All fivecalloc()calls in the file are now guarded.Closes #565 — newline written past
flogmessage. The twostrncat()calls are bounded so the message can fillLOGSIZEexactly, after whichstrcat()put the newline atLOGSIZE-1and its terminator atLOGSIZE. The newline is now appended only when a byte is free, and replaces the final character otherwise. Three cmocka cases cover it, with a canary after the buffer so an overflow is observable rather than merely undefined.Closes #566 —
MYSQL_RESleaked on the no-row branch.getsetting(),getpsetting(),getglobalvariable()and the Cacti version read each returned from themysql_fetch_row() == NULLbranch without freeing, while every other exit in those functions frees.Verification. Clean
ubuntu:24.04container: builds at 4 warnings, matching develop exactly.make checkpasses all three binaries,make distcheckpasses.Not folded into #555. That PR is ping-specific and currently sits on a stale "merge conflicts" block that only needs lifting; adding unrelated spine.c and util.c changes would force a fresh review of work that is otherwise ready.
The 1.2.x half of these is going onto #574, since all three are present there too.