Improve rpc msg handling#459
Merged
carlgsmith merged 3 commits intoJun 5, 2026
Merged
Conversation
rpc_msg_decode_string() only checked that the read offset was within the message before calling strlen() on the bytes there. The receive buffer is sized to the number of bytes received and is not guaranteed to be NUL padded, so a message whose final string is not NUL terminated caused strlen() to read past the end of the heap allocation (out-of-bounds read). Apteryx accepts messages over TCP as well as unix sockets, so this is reachable from untrusted peers. Require the string to be NUL terminated within the remaining message bytes (memchr); otherwise treat the message as malformed and return NULL. Adds a test that decodes an unterminated buffer sized past the minimum allocation - this reliably trips AddressSanitizer without the bound. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When decoding a tree, an rpc_value marker not followed by a key string left key == NULL, which was then passed to g_strdup()/strrchr() and dereferenced - a single truncated message (e.g. one trailing rpc_value byte) crashed the daemon while decoding a SET/QUERY. As this is reachable from any peer it is a denial of service. Treat a missing key as end-of-input and return what has been decoded so far. Adds a test decoding a lone rpc_value marker. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_rpc_msg_decode_tree() recursed once per rpc_start_children marker with no limit, so a small message consisting of many nested markers could exhaust the stack and crash the daemon - a denial of service reachable from any peer. Thread a depth counter through the recursion and stop at RPC_MSG_MAX_TREE_DEPTH (1024), far beyond any real configuration tree. Decoding then unwinds and returns the partial tree instead of overflowing. Adds a test that decodes a message of 200k nesting markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Handle some edge cases in the rpc message processing