Skip to content

fix(network): guard empty buffer in UpgradeStatusExtension::decode#399

Open
mark0-cn wants to merge 1 commit into
bnb-chain:mainfrom
mark0-cn:mark0/fix/upgrade-status-empty-buf
Open

fix(network): guard empty buffer in UpgradeStatusExtension::decode#399
mark0-cn wants to merge 1 commit into
bnb-chain:mainfrom
mark0-cn:mark0/fix/upgrade-status-empty-buf

Conversation

@mark0-cn

Copy link
Copy Markdown

Summary

The BSC handshake decodes a peer-controlled buffer into UpgradeStatusExtension, but UpgradeStatusExtension::decode (src/node/network/upgrade_status.rs) indexed buf[0] without an emptiness check. A truncated or empty UpgradeStatus message therefore panics the connection/handshake task with an index-out-of-bounds — and the buffer comes straight off the wire during the handshake, so any peer can trigger it.

The sibling BscCapPacket::decode already guards buf.is_empty() and returns alloy_rlp::Error::InputTooShort; this applies the same guard here.

Added a regression test asserting that decoding an empty buffer returns Err instead of panicking.

The BSC handshake decodes a peer-controlled buffer, but `decode` indexed
`buf[0]` without an emptiness check, so a truncated/empty UpgradeStatus message
panicked the connection task — any peer could trigger it. Guard `buf.is_empty()`
and return `InputTooShort`, matching the sibling `BscCapPacket::decode`.
@mark0-cn
mark0-cn requested a review from joey0612 as a code owner June 22, 2026 10:21
@hashdit-bot

hashdit-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

Pull Request Review

This PR fixes a panic condition in the Rust P2P handshake decoder by adding an is_empty() guard before indexing buf[0] in UpgradeStatusExtension::decode. Previously, an empty or truncated peer-controlled buffer could cause an index-out-of-bounds panic; now it returns alloy_rlp::Error::InputTooShort instead. A regression unit test was also added to ensure empty-buffer decoding returns an error rather than panicking.

Sensitive Content

No sensitive content detected.

Security Issues

No serious security issues detected.


Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits.

@mark0-cn

Copy link
Copy Markdown
Author

@joey0612

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