dbfile: zero-initialise the uuid buffer in __dbfile_get_config - #414
Open
martinus wants to merge 1 commit into
Open
dbfile: zero-initialise the uuid buffer in __dbfile_get_config#414martinus wants to merge 1 commit into
martinus wants to merge 1 commit into
Conversation
__dbfile_get_config() read an uninitialised buffer. get_config_text() memcpy()s exactly `len` (36) bytes into the buffer without a NUL terminator - and writes nothing at all when the config row is absent - so uuid_parse()'s strlen() ran past the 36-byte uuid into uninitialised stack, and on a missing config row could parse garbage over the default fs_uuid. Zero-initialise the buffer so it is always terminated. Harmless in practice (the trailing stack bytes were usually zero), but it's a real uninitialised read - valgrind flags it on config load. Co-Authored-By: Claude Opus 4.8 <[email protected]>
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.
What this fixes
__dbfile_get_config()declareschar uuid[37];uninitialised, then fills it viaget_config_text(stmt, "fs_uuid", uuid, 36).get_config_text()memcpy()s exactlylen(36) bytes without writing a NUL terminator — and writes nothing at all if thefs_uuidconfig row is absent. It then callsuuid_parse(uuid, ...), whichstrlen()s its argument.What happens without the fix
uuid_parse()'sstrlen()runs past the 36-byte uuid into uninitialised stack (an uninitialised read — valgrind flags it on every config load). If thefs_uuidrow is missing, the whole buffer is uninitialised anduuid_parse()can parse garbage over the defaultfs_uuid. In practice the trailing stack bytes are usually zero, so it silently "works" — but it's undefined behaviour.The fix
Zero-initialise the buffer (
char uuid[37] = "";) so it is always NUL-terminated regardless of whatget_config_text()wrote.Found via a valgrind sweep in the
oansfork; the defect is inherited straight from here.