Reject undersized UDP packets before reading header fields#15
Open
acts-1631 wants to merge 1 commit into
Open
Conversation
ReceiveInternal calls InitSelectRead (recvfrom) into a stack-allocated BibleSyncMessage and immediately formats the uuid and body into a diagnostic dump that is passed to the application's nav_func callback. There was no minimum length check, so a packet shorter than BSP_HEADER_SIZE (32 bytes) leaves the uuid field (offsets 16-31) and body (offset 32+) as uninitialized stack memory, which then gets formatted into the dump and handed to the application. MemorySanitizer confirms use-of-uninitialized-value when reading the uuid field after a 6-byte recvfrom. Add a recv_size < BSP_HEADER_SIZE check immediately after recvfrom returns. Undersized packets are reported as an error via nav_func and skipped before any uninitialized header field is read.
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.
ReceiveInternal formats the received packet's uuid and body into a
diagnostic dump that is passed to the application's nav_func callback,
but there was no minimum length check after recvfrom. A packet shorter
than BSP_HEADER_SIZE (32 bytes) leaves the uuid field and body as
uninitialized stack memory, which then gets formatted into the dump.
MemorySanitizer confirms use-of-uninitialized-value on the uuid read
after a 6-byte recvfrom.
This adds a recv_size < BSP_HEADER_SIZE check right after recvfrom
returns. Undersized packets are reported as an error via nav_func and
skipped before any uninitialized header field is read.