Fix NULL pointer deref and memory leak in CSEXP parser#366
Merged
vstakhov merged 1 commit intovstakhov:masterfrom Mar 24, 2026
Merged
Fix NULL pointer deref and memory leak in CSEXP parser#366vstakhov merged 1 commit intovstakhov:masterfrom
vstakhov merged 1 commit intovstakhov:masterfrom
Conversation
Two coupled bugs in ucl_parse_csexp: 1. NULL pointer dereference in read_value (line 181): after closing the outermost list with ')', parser->stack becomes NULL. If trailing data follows (e.g. input "()3:abc"), the parser reaches read_value and dereferences parser->stack->obj without a NULL check. Minimal trigger: ()3:abc — found via libFuzzer + ASan. 2. Memory leak on error paths: ucl_parser_free frees stack frame structs but not the ucl_object_t they hold. In the CSEXP parser, stack objects are only appended to their parent on ')'. If parsing fails with unclosed lists, inner stack objects are never appended to the tree and are permanently leaked. Minimal trigger: (( — confirmed via macOS leaks tool: 18 leaks / 1152 bytes before fix, 0 after. These are coupled because fix vstakhov#1 sends execution to the parse_err state, and fix vstakhov#2 is the cleanup logic that runs there. Fix vstakhov#1: add NULL check for parser->stack before ucl_array_append. Fix vstakhov#2: in parse_err and the "invalid finishing state" error path, iterate remaining stack frames and ucl_object_unref any object that is not parser->top_obj (orphaned objects not reachable from the tree).
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
2 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.
Fix #365, including two coupled bugs in ucl_parse_csexp:
NULL pointer dereference in read_value (line 181): after closing the outermost list with ')', parser->stack becomes NULL. If trailing data follows (e.g. input "()3:abc"), the parser reaches read_value and dereferences parser->stack->obj without a NULL check. Minimal trigger: ()3:abc — found via libFuzzer + ASan.
Memory leak on error paths: ucl_parser_free frees stack frame structs but not the ucl_object_t they hold. In the CSEXP parser, stack objects are only appended to their parent on ')'. If parsing fails with unclosed lists, inner stack objects are never appended to the tree and are permanently leaked. Minimal trigger: (( — confirmed via macOS leaks tool: 18 leaks / 1152 bytes before fix, 0 after.
These are coupled because fix_1 sends execution to the parse_err state, and fix_2 is the cleanup logic that runs there.