load_ini_buffer: allocate ini_length + 1 bytes (fixes OOB write) - #21
Open
1820893135-pixel wants to merge 1 commit into
Open
load_ini_buffer: allocate ini_length + 1 bytes (fixes OOB write)#211820893135-pixel wants to merge 1 commit into
1820893135-pixel wants to merge 1 commit into
Conversation
`strip_ini_cache()` documents that the `ini_source` buffer must be able to
hold `ini_length + 1` bytes, since the first thing it does is
ini_source[ini_length] = '\0';
regardless of the input. The bundled `load_ini_buffer()` helper allocated
only `ini_length` bytes and then handed the buffer to `strip_ini_cache()`,
so every non-empty input wrote one byte past the allocation (an ASan
heap-buffer-overflow on the NUL terminator).
Allocate `ini_length + 1` bytes to match the documented contract.
Reproducer (ASan): calling `load_ini_buffer("[S]\nkey = value\n", ...)`
aborts with "heap-buffer-overflow ... WRITE of size 1 ... in strip_ini_cache"
before the fix and runs cleanly afterwards.
Fixes madmurphy#20.
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
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.
Fixes the heap-buffer-overflow reported in #20.
strip_ini_cache()documents (and does) a mandatory one-byte write:so its
ini_sourceargument needsini_length + 1usable bytes. The bundled helperexamples/utilities/load_ini_buffer.hviolated that contract:Every non-empty input therefore wrote one NUL byte past the allocation. Other in-tree callers are fine —
examples/topics/strip_ini_cache.cusesstrndup()and the two doc examples insrc/confini.cusemalloc(file_size + 1)— this helper was the only one missing the extra byte.Fix: allocate
ini_length + 1bytes to match the documented contract.Verification
ASan reproducer using the bundled header itself:
clang -fsanitize=address -I src -I examples/utilities test.c src/confini.c -o testBefore: aborts with
After: exits 0 with no sanitizer report.
I also checked the UTF-8 BOM probe at
src/confini.c:2636-2640(ini_source[0..2]): it is not separately exploitable, because the mandatoryini_source[ini_length] = '\0'always terminates that three-byte chain first (ini_source[2]is only read whenini_length >= 3, in which case it is in bounds). So the allocation is the only change needed.Fixes #20.