Skip to content

check chunk header bounds in ubring recv paths - #3497

Open
ubeddulla wants to merge 1 commit into
apache:masterfrom
ubeddulla:ubring-recv-chunk-bounds
Open

check chunk header bounds in ubring recv paths#3497
ubeddulla wants to merge 1 commit into
apache:masterfrom
ubeddulla:ubring-recv-chunk-bounds

Conversation

@ubeddulla

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve N/A

Problem Summary:

UbrTrxRecvBlockMode and StartReadv read a chunk's msg_len and cur_index straight from the receive-ring header that the remote peer writes into shared memory, then memcpy msg_len minus cur_index bytes out of the fixed 60-byte payload.inner without checking either value. A peer that sets msg_len above UBR_MSG_PAYLOAD_LEN (60), or cur_index above msg_len so the uint8_t subtraction wraps, makes that copy read past the payload into the adjacent chunk header and neighbouring ring slots. Under ASAN this shows up as a heap-buffer-overflow read of size 255 at 0 bytes after the 64-byte chunk. The send path already caps msg_len at UBR_MSG_PAYLOAD_LEN, so only the receive side was missing the check.

What is changed and the side effects?

Changed:

Added IsRecvChunkHeaderValid (msg_len <= UBR_MSG_PAYLOAD_LEN and cur_index <= msg_len) and apply it at both receive sites before the copy, rejecting a malformed chunk with errno=EBADMSG and the same UBRING_ERR the existing pre-checks return. Added a regression test in brpc_ubring_unittest.cpp.

Side effects:

  • Performance effects: one comparison per chunk, negligible.

  • Breaking backward compatibility: none. Valid chunks (msg_len <= 60, cur_index <= msg_len) behave exactly as before.


Check List:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant