bug: `assert!()` on peer-supplied `payload_len` in P2P validation allows remote node crash

[huth-stacks]

Bug Description

Five validate_*_push() functions in stackslib/src/net/chat.rs use assert!() to validate preamble.payload_len — a field supplied by remote peers. In Rust, assert!() panics in all builds (not just debug builds; that's debug_assert!()). A malicious peer can crash any node by sending a message with a small payload_len.

This directly violates the codebase's own coding guideline from CONTRIBUTING.md:

Functions that act on externally-submitted data must never panic. This includes code that acts on incoming network messages, blockchain data, and burnchain (Bitcoin) data.

Affected Functions

Function	Line	Assert	Threshold
validate_blocks_push	2079	assert!(preamble.payload_len > 5)	5
validate_microblocks_push	2119	assert!(preamble.payload_len > 5)	5
validate_transaction_push	2156	assert!(preamble.payload_len > 1)	1
validate_stackerdb_push	2194	assert!(preamble.payload_len > 1)	1
validate_nakamoto_block_push	2233	assert!(preamble.payload_len > 1)	1

Example from validate_blocks_push:

fn validate_blocks_push(
    &mut self,
    preamble: &Preamble,
    relayers: Vec<RelayData>,
) -> Result<Option<StacksMessage>, net_error> {
    assert!(preamble.payload_len > 5); // PANICS on malicious input
    // ...
}

Impact

Any peer on the network can remotely crash a Stacks node by sending a crafted P2P message with payload_len <= 5 (or <= 1 for the latter three functions). This is a denial-of-service vulnerability requiring no authentication — just a TCP connection to the node's P2P port.

Recommended Fix

Replace each assert!() with an if check that returns Err(net_error::InvalidMessage). The InvalidMessage variant is a unit variant already used throughout the P2P layer for this purpose.

- assert!(preamble.payload_len > 5);
+ if preamble.payload_len <= 5 {
+     return Err(net_error::InvalidMessage);
+ }

Apply the same pattern to all five functions, using the appropriate threshold (5 or 1).

The caller at chat.rs:2996 already propagates net_error via ?, so the error will be handled gracefully — the malformed message is dropped and the peer connection continues.

Suggested Test

For each of the five functions, construct a Preamble with payload_len at or below the threshold and call the validation function. Assert that it returns Err(net_error::InvalidMessage) rather than panicking.

---

Agent Prompt

Fix the remote crash vulnerability in stackslib/src/net/chat.rs where assert!()
is used on peer-supplied data.

Five validate_*_push() functions use assert!() to check preamble.payload_len,
which panics on malicious input. Per CONTRIBUTING.md, functions acting on
externally-submitted data must never panic.

Fix:
1. In each of these 5 functions, replace the assert!() with a check that returns
   Err(net_error::InvalidMessage):
   - validate_blocks_push (line 2079): assert!(preamble.payload_len > 5)
   - validate_microblocks_push (line 2119): assert!(preamble.payload_len > 5)
   - validate_transaction_push (line 2156): assert!(preamble.payload_len > 1)
   - validate_stackerdb_push (line 2194): assert!(preamble.payload_len > 1)
   - validate_nakamoto_block_push (line 2233): assert!(preamble.payload_len > 1)

   Pattern:
   if preamble.payload_len <= THRESHOLD {
       return Err(net_error::InvalidMessage);
   }

2. net_error::InvalidMessage is a unit variant (no parameters needed).

3. Add tests for each function that construct a Preamble with payload_len at or
   below the threshold and verify Err(net_error::InvalidMessage) is returned
   instead of a panic.

4. Run `cargo test -p stackslib` to verify.

[brice-stacks]

I believe Preamble::consensus_deserialize ensures that this is not reachable, but I agree it would be appropriate to change these from asserts to errors.