Repository navigation
cblockd: fix heap overflow when replaying console history - #131
Merged
Merged
Conversation
tty_trim_buffer() strips trailing whitespace and NULs from the console history before it is replayed to a newly attached client. If the scan never found a non-whitespace byte above input[0] (for example a history of just "\r\n"), the "new" pointer was left at 0 and the returned length was computed as len - (pointer - 0), which wraps to a huge size_t. dispatch_connect_console() then sent 64K frames past the end of the termbuf_to_contig() allocation, which crashed cblockd under ASAN and could send heap contents to the console client otherwise. Rewrite the function to count the length down instead of doing pointer arithmetic. This also checks input[0], which the old loop skipped, and casts to unsigned char before calling isspace(), because passing a negative char is undefined behaviour for bytes >= 0x80. Writing the terminating NUL at input[len] is safe because termbuf_to_contig() allocates t_tot_len + 1 bytes.
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.
tty_trim_buffer() strips trailing whitespace and NULs from the console history before it is replayed to a newly attached client. If the scan never found a non-whitespace byte above input[0] (for example a history of just "\r\n"), the "new" pointer was left at 0 and the returned length was computed as len - (pointer - 0), which wraps to a huge size_t. dispatch_connect_console() then sent 64K frames past the end of the termbuf_to_contig() allocation, which crashed cblockd under ASAN and could send heap contents to the console client otherwise.
Rewrite the function to count the length down instead of doing pointer arithmetic. This also checks input[0], which the old loop skipped, and casts to unsigned char before calling isspace(), because passing a negative char is undefined behaviour for bytes >= 0x80.
Writing the terminating NUL at input[len] is safe because termbuf_to_contig() allocates t_tot_len + 1 bytes.