[message] Deserializer and sanitizer support massively oversized V1 transactions

[robertkiel]

Requires #832

SIMD 385 bumps the transaction size limit to 4096 bytes. However, neither the deserializer nor the sanitize() function enforce this limit. This allows a semi-honest user of the platform to submit oversized transactions. See PoC for a transaction that passes sanitize() with a size of ~4.2 MiB.

Details

The code specifies a maximum transaction size but apparently, this constant is only used in tests and not enforced by the deserializer or the sanitizer.

Code

/// Maximum transaction size for V1 format in bytes.
pub const MAX_TRANSACTION_SIZE: usize = 4096;

Suggestion

By utilizing wincode's deserialize_exact API, the deserializer automatically rejects trailing bytes and defines an upper bound for DST-allocations, such as Vec<T>. This does not enforce the exact upper bound of 4096 bytes per transaction but provides an easy and effective way to safe the network from handling and filtering oversized transactions.

Code

pub fn deserialize(input: &[u8]) -> wincode::ReadResult<Message> {
    wincode::config::deserialize_exact(input, wincode::config::DefaultConfig::default().with_preallocation_size_limit::<MAX_TRANSACTION_SIZE>())
}

PoC

Add to message/tests/v1_sanitize_bypass.rs

//! Exercises the V1 `Message::validate` / `sanitize` gate and shows what it does
//! *not* catch.
//!
//! `validate()` (message/src/versions/v1/message.rs:434) bounds the account and
//! instruction *counts* (<= 64 each) and every account/program index, but it
//! never bounds the *total serialized size*. The documented per-transaction cap
//! `MAX_TRANSACTION_SIZE = 4096` (v1/mod.rs:36) is a dead constant — it is never
//! referenced by `validate`, `sanitize`, or the deserializer.
//!
//! Each of 64 instructions may carry up to 255 account-index bytes and 65_535
//! data bytes, so a message that passes `sanitize()` cleanly can still be ~4 MiB
//! — roughly 1000x the 4 KiB limit the format was designed around.

use {
    solana_address::Address,
    solana_hash::Hash,
    solana_message::{
        compiled_instruction::CompiledInstruction,
        v1::{Message, TransactionConfig, MAX_ADDRESSES, MAX_INSTRUCTIONS, MAX_TRANSACTION_SIZE},
        MessageHeader,
    },
    solana_sanitize::Sanitize,
};

/// Builds a fully valid (sanitize-passing) V1 message that is as large as the
/// count/index checks allow: 64 addresses, 64 instructions, each instruction
/// with 255 account-index bytes and 65_535 data bytes.
fn build_oversized_but_valid_message() -> Message {
    let num_addresses = MAX_ADDRESSES; // 64
    let num_instructions = MAX_INSTRUCTIONS as usize; // 64

    // 64 unique addresses so the duplicate check passes.
    let account_keys: Vec<Address> = (0..num_addresses).map(|_| Address::new_unique()).collect();

    // Every account index points at key 1 (valid: 0 < 1 < 64, and != fee payer 0).
    let instruction = CompiledInstruction {
        program_id_index: 1,
        accounts: vec![1u8; u8::MAX as usize], // 255 index bytes (dupes allowed)
        data: vec![0u8; u16::MAX as usize],    // 65_535 data bytes
    };
    let instructions = vec![instruction; num_instructions];

    Message {
        header: MessageHeader {
            num_required_signatures: 1,
            num_readonly_signed_accounts: 0,
            num_readonly_unsigned_accounts: 1,
        },
        config: TransactionConfig::empty(),
        lifetime_specifier: Hash::default(),
        account_keys,
        instructions,
    }
}

#[test]
fn sanitize_passes_but_size_exceeds_max_transaction_size() {
    let message = build_oversized_but_valid_message();

    // The count/index checks are all satisfied...
    message
        .sanitize()
        .expect("oversized message still passes sanitize()");

    // ...yet the serialized message dwarfs the documented 4 KiB limit, which
    // sanitize never enforces.
    let size = message.size();
    assert!(
        size > MAX_TRANSACTION_SIZE,
        "expected {size} > MAX_TRANSACTION_SIZE ({MAX_TRANSACTION_SIZE})"
    );

    // Concretely: 64 * (4 header + 255 accounts + 65_535 data) + 64*32 addrs + 41 fixed.
    // ~4.2 MiB -> more than 1000x the 4096-byte cap.
    assert!(size > 4_000_000, "sanitized message is ~4 MiB: {size}");
    assert!(size > 1000 * MAX_TRANSACTION_SIZE);
}

[robertkiel]

Related code in agave, using deserialize_exact and an allocation limit

pub(crate) fn deserialize_request<T>(
    request: &BytesPacket,
) -> std::result::Result<T, wincode::ReadError>
where
    T: for<'de> SchemaRead<'de, PacketConfig, Dst = T>,
{
    wincode::config::deserialize_exact(request.buffer(), PacketConfig::new())
}