Skip to content

Commit cf11c00

Browse files
authored
program: safe deserialize instruction (#42)
1 parent 3821e81 commit cf11c00

File tree

5 files changed

+161
-11
lines changed

5 files changed

+161
-11
lines changed

Cargo.lock

Lines changed: 21 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

program/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ solana-program = "2.0.1"
2828
mollusk-svm = { version = "0.0.5", features = ["fuzz"] }
2929
mollusk-svm-bencher = "0.0.5"
3030
solana-sdk = "2.0.1"
31+
strum = "0.24"
32+
strum_macros = "0.24"
3133
test-case = "3.3.1"
3234

3335
[lib]

program/benches/compute_units.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
#### Compute Units: 2024-11-07 16:24:48.059441548 UTC
2+
3+
| Name | CUs | Delta |
4+
|------|------|-------|
5+
| create_lookup_table | 10758 | +10 |
6+
| freeze_lookup_table | 1517 | +7 |
7+
| extend_lookup_table_from_0_to_1 | 6222 | +6 |
8+
| extend_lookup_table_from_0_to_10 | 8841 | -39 |
9+
| extend_lookup_table_from_0_to_38 | 17077 | -179 |
10+
| extend_lookup_table_from_1_to_2 | 6222 | +6 |
11+
| extend_lookup_table_from_1_to_10 | 8547 | -34 |
12+
| extend_lookup_table_from_1_to_39 | 17077 | -179 |
13+
| extend_lookup_table_from_5_to_6 | 6222 | +6 |
14+
| extend_lookup_table_from_5_to_15 | 8842 | -39 |
15+
| extend_lookup_table_from_5_to_43 | 17077 | -179 |
16+
| extend_lookup_table_from_25_to_26 | 6225 | +6 |
17+
| extend_lookup_table_from_25_to_35 | 8844 | -39 |
18+
| extend_lookup_table_from_25_to_63 | 17080 | -179 |
19+
| extend_lookup_table_from_50_to_88 | 17083 | -179 |
20+
| extend_lookup_table_from_100_to_138 | 17089 | -179 |
21+
| extend_lookup_table_from_150_to_188 | 17096 | -179 |
22+
| extend_lookup_table_from_200_to_238 | 17102 | -179 |
23+
| extend_lookup_table_from_255_to_256 | 6254 | +6 |
24+
| deactivate_lookup_table | 3151 | +8 |
25+
| close_lookup_table | 2226 | +6 |
26+
127
#### Compute Units: 2024-10-22 17:54:37.721198 UTC
228

329
| Name | CUs | Delta |

program/src/instruction.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use {
1010
},
1111
};
1212

13+
#[cfg_attr(test, derive(strum_macros::EnumIter))]
1314
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
1415
pub enum AddressLookupTableInstruction {
1516
/// Create an address lookup table

program/src/processor.rs

Lines changed: 111 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,61 @@ fn get_lookup_table_status(
6060
}
6161
}
6262

63-
// [Core BPF]: Locally-implemented
64-
// `solana_sdk::program_utils::limited_deserialize`.
65-
fn limited_deserialize<T>(input: &[u8]) -> Result<T, ProgramError>
66-
where
67-
T: serde::de::DeserializeOwned,
68-
{
69-
solana_program::program_utils::limited_deserialize(
70-
input, 1232, // [Core BPF]: See `solana_sdk::packet::PACKET_DATA_SIZE`
71-
)
72-
.map_err(|_| ProgramError::InvalidInstructionData)
63+
// Maximum input buffer length that can be deserialized.
64+
// See `solana_sdk::packet::PACKET_DATA_SIZE`.
65+
const MAX_INPUT_LEN: usize = 1232;
66+
// Maximum vector length for new keys to be appended to a lookup table,
67+
// provided to the `ExtendLookupTable` instruction.
68+
// See comments below for `safe_deserialize_instruction`.
69+
//
70+
// Take the maximum input length and subtract 4 bytes for the discriminator,
71+
// 8 bytes for the vector length, then divide that by the size of a `Pubkey`.
72+
const MAX_NEW_KEYS_VECTOR_LEN: usize = (MAX_INPUT_LEN - 4 - 8) / 32;
73+
74+
// Stub of `AddressLookupTableInstruction` for partial deserialization.
75+
// Keep in sync with the program's instructions in `instructions`.
76+
#[allow(clippy::enum_variant_names)]
77+
#[cfg_attr(test, derive(strum_macros::EnumIter))]
78+
#[derive(serde::Serialize, serde::Deserialize, PartialEq)]
79+
enum InstructionStub {
80+
CreateLookupTable,
81+
FreezeLookupTable,
82+
ExtendLookupTable { vector_len: u64 },
83+
DeactivateLookupTable,
84+
CloseLookupTable,
85+
}
86+
87+
// [Core BPF]: The original Address Lookup Table builtin leverages the
88+
// `solana_sdk::program_utils::limited_deserialize` method to cap the length of
89+
// the input buffer at `MAX_INPUT_LEN` (1232). As a result, any input buffer
90+
// larger than `MAX_INPUT_LEN` will abort deserialization and return
91+
// `InstructionError::InvalidInstructionData`.
92+
//
93+
// Howevever, since `ExtendLookupTable` contains a vector of `Pubkey`, the
94+
// `limited_deserialize` method will still read the vector's length and attempt
95+
// to allocate a vector of the designated size. For extremely large length
96+
// values, this can cause the initial allocation of a large vector to exhuast
97+
// the BPF program's heap before deserialization can proceed.
98+
//
99+
// To mitigate this memory issue, the BPF version of the program has been
100+
// designed to "peek" the length value for `ExtendLookupTable`, and ensure it
101+
// cannot allocate a vector that would otherwise violate the input buffer
102+
// length restriction.
103+
fn safe_deserialize_instruction(
104+
input: &[u8],
105+
) -> Result<AddressLookupTableInstruction, ProgramError> {
106+
match bincode::deserialize::<InstructionStub>(input)
107+
.map_err(|_| ProgramError::InvalidInstructionData)?
108+
{
109+
InstructionStub::ExtendLookupTable { vector_len }
110+
if vector_len as usize > MAX_NEW_KEYS_VECTOR_LEN =>
111+
{
112+
return Err(ProgramError::InvalidInstructionData);
113+
}
114+
_ => {}
115+
}
116+
solana_program::program_utils::limited_deserialize(input, MAX_INPUT_LEN as u64)
117+
.map_err(|_| ProgramError::InvalidInstructionData)
73118
}
74119

75120
// [Core BPF]: Feature "FKAcEvNgSY79RpqsPNUV5gDyumopH4cEHqUxyfm8b8Ap"
@@ -461,7 +506,7 @@ fn process_close_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo]) ->
461506
/// Processes a
462507
/// `solana_programs_address_lookup_table::instruction::AddressLookupTableInstruction`
463508
pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult {
464-
let instruction = limited_deserialize(input)?;
509+
let instruction = safe_deserialize_instruction(input)?;
465510
match instruction {
466511
AddressLookupTableInstruction::CreateLookupTable {
467512
recent_slot,
@@ -488,3 +533,58 @@ pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> P
488533
}
489534
}
490535
}
536+
537+
#[cfg(test)]
538+
mod tests {
539+
use super::*;
540+
541+
fn assert_instruction_serialization(
542+
stub: &InstructionStub,
543+
instruction: &AddressLookupTableInstruction,
544+
len: usize,
545+
) {
546+
assert_eq!(
547+
bincode::serialize(&stub).unwrap(),
548+
bincode::serialize(&instruction).unwrap()[0..len],
549+
)
550+
}
551+
552+
#[test]
553+
fn test_instruction_stubs() {
554+
assert_eq!(
555+
<InstructionStub as strum::IntoEnumIterator>::iter().count(),
556+
<AddressLookupTableInstruction as strum::IntoEnumIterator>::iter().count(),
557+
);
558+
559+
assert_instruction_serialization(
560+
&InstructionStub::CreateLookupTable,
561+
&AddressLookupTableInstruction::CreateLookupTable {
562+
recent_slot: 0,
563+
bump_seed: 0,
564+
},
565+
4,
566+
);
567+
assert_instruction_serialization(
568+
&InstructionStub::FreezeLookupTable,
569+
&AddressLookupTableInstruction::FreezeLookupTable,
570+
4,
571+
);
572+
assert_instruction_serialization(
573+
&InstructionStub::ExtendLookupTable { vector_len: 4 },
574+
&AddressLookupTableInstruction::ExtendLookupTable {
575+
new_addresses: vec![Pubkey::new_unique(); 4],
576+
},
577+
12, // Check the vector length as well.
578+
);
579+
assert_instruction_serialization(
580+
&InstructionStub::DeactivateLookupTable,
581+
&AddressLookupTableInstruction::DeactivateLookupTable,
582+
4,
583+
);
584+
assert_instruction_serialization(
585+
&InstructionStub::CloseLookupTable,
586+
&AddressLookupTableInstruction::CloseLookupTable,
587+
4,
588+
);
589+
}
590+
}

0 commit comments

Comments
 (0)