anza-xyz / anza-xyz/solana-sdk

[message] Deserializer and sanitizer support massively oversized V1 transactions

Abierto
#831 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Rust
Estrellas
256
Forks
250
Merge medio
2 d 3 h
PR fusionados (30 d)
39

Descripción

_Requires #832_

[SIMD 385](https://github.com/solana-foundation/solana-improvement-documents/blob/main/proposals/0385-transaction-v1.md) 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](https://github.com/anza-xyz/solana-sdk/blob/33e4209a50cfadd6f7be2f5323d7380c8a537843/message/src/versions/v1/mod.rs#L35-L36)
```rust
/// Maximum transaction size for V1 format in bytes.
pub const MAX_TRANSACTION_SIZE: usize = 4096;
```

### Suggestion

By utilizing wincode's `deserialize_exact` [API](https://docs.rs/wincode/latest/wincode/config/fn.deserialize_exact.html), the deserializer automatically rejects trailing bytes and defines an upper bound for DST-allocations, such as `Vec`. 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](https://github.com/anza-xyz/solana-sdk/blob/5a0be40fca67bbd5c1b2c3a3e9c825c62538bb2d/message/src/versions/v1/message.rs#L709-L715)
```rust
pub fn deserialize(input: &[u8]) -> wincode::ReadResult {
wincode::config::deserialize_exact(input, wincode::config::DefaultConfig::default().with_preallocation_size_limit::())
}
```

### PoC

Add to `message/tests/v1_sanitize_bypass.rs`

```rust
//! 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

= (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);
}
```

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.