program: safe deserialize config keys

Author: buffalojoec

program/src/processor.rs

@@ -12,21 +12,43 @@ use {
     std::collections::BTreeSet,
 };
 
-// [Core BPF]: Locally-implemented
-// `solana_sdk::program_utils::limited_deserialize`.
-fn limited_deserialize<T>(input: &[u8]) -> Result<T, ProgramError>
-where
-    T: serde::de::DeserializeOwned,
-{
-    solana_program::program_utils::limited_deserialize(
-        input, 1232, // [Core BPF]: See `solana_sdk::packet::PACKET_DATA_SIZE`
-    )
-    .map_err(|_| ProgramError::InvalidInstructionData)
+// [Core BPF]: The original Config builtin leverages the
+// `solana_sdk::program_utils::limited_deserialize` method to cap the length of
+// the input buffer at 1232 (`solana_sdk::packet::PACKET_DATA_SIZE`). As a
+// result, any input buffer larger than 1232 will abort deserialization and
+// return `InstructionError::InvalidInstructionData`.
+//
+// Howevever, since `ConfigKeys` contains a vector of `(Pubkey, bool)`, the
+// `limited_deserialize` method will still read the vector's length and attempt
+// to allocate a vector of the designated size. For extremely large length
+// values, this can cause the initial allocation of a large vector to exhuast
+// the BPF program's heap before deserialization can proceed.
+//
+// To mitigate this memory issue, the BPF version of the Config program has
+// been designed to "peek" the length value, and ensure it cannot allocate a
+// vector that would otherwise violate the input buffer length restriction of
+// 1232.
+//
+// Taking the maximum input length of 1232 and subtracting (up to) 3 bytes for
+// the `ShortU16`, then dividing that by the size of a `(Pubkey, bool)` entry
+// (33), we get a maximum vector size of 37. A `ShortU16` value for 37 fits in
+// just one byte (`[0x25]`), so this function can simply check the first
+// provided byte.
+fn safe_deserialize_config_keys(input: &[u8]) -> Result<ConfigKeys, ProgramError> {
+    match input.first() {
+        Some(first_byte) if *first_byte <= 0x25 => {
+            solana_program::program_utils::limited_deserialize::<ConfigKeys>(
+                input, 1232, // [Core BPF]: See `solana_sdk::packet::PACKET_DATA_SIZE`
+            )
+            .map_err(|_| ProgramError::InvalidInstructionData)
+        }
+        _ => Err(ProgramError::InvalidInstructionData),
+    }
 }
 
 /// Config program processor.
 pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult {
-    let key_list: ConfigKeys = limited_deserialize(input)?;
+    let key_list = safe_deserialize_config_keys(input)?;
 
     let mut accounts_iter = accounts.iter();
     let config_account = next_account_info(&mut accounts_iter)?;

program/tests/functional.rs

@@ -587,3 +587,65 @@ fn test_maximum_keys_input() {
         &[Check::err(ProgramError::InvalidInstructionData)],
     );
 }
+
+#[test]
+fn test_safe_deserialize() {
+    let mollusk = setup();
+
+    // Accounts don't matter for this test.
+
+    // First try to spoof the program with just `ShortU16` length values.
+    let build_instruction =
+        |data: &[u8]| Instruction::new_with_bytes(solana_config_program::id(), data, vec![]);
+
+    mollusk.process_and_validate_instruction(
+        // Empty buffer. Not a valid `ShortU16`.
+        &build_instruction(&[]),
+        &[],
+        &[Check::err(ProgramError::InvalidInstructionData)],
+    );
+
+    mollusk.process_and_validate_instruction(
+        // `ShortU16` value of 38. One byte too large.
+        &build_instruction(&[0x26]),
+        &[],
+        &[Check::err(ProgramError::InvalidInstructionData)],
+    );
+
+    mollusk.process_and_validate_instruction(
+        // `ShortU16` value of 37. OK for vector size, but no keys following.
+        &build_instruction(&[0x25]),
+        &[],
+        &[Check::err(ProgramError::InvalidInstructionData)],
+    );
+
+    // Now try with some actual `ConfigKeys` inputs.
+    let mut keys = Vec::new();
+    let serialized_config_keys = |keys: &[(Pubkey, bool)]| {
+        let config_keys = ConfigKeys {
+            keys: keys.to_vec(),
+        };
+        bincode::serialize(&config_keys).unwrap()
+    };
+
+    // First build out to an acceptable size of 37.
+    (0..37).for_each(|i| keys.push((Pubkey::new_unique(), i % 2 == 0)));
+
+    mollusk.process_and_validate_instruction(
+        // `ShortU16` value of 37. OK.
+        &build_instruction(&serialized_config_keys(&keys)),
+        &[],
+        // Falls through to account keys failure.
+        &[Check::err(ProgramError::NotEnoughAccountKeys)],
+    );
+
+    // Add one more key, pushing the size to 38.
+    keys.push((Pubkey::new_unique(), true));
+
+    mollusk.process_and_validate_instruction(
+        // `ShortU16` value of 38. Err.
+        &build_instruction(&serialized_config_keys(&keys)),
+        &[],
+        &[Check::err(ProgramError::InvalidInstructionData)],
+    );
+}