From 9ca26f30dcbf005dc86428d4f46adb4db8393ae0 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Mon, 4 Nov 2024 15:01:24 +0400 Subject: [PATCH 1/2] program: safe deserialize config keys from state --- program/src/processor.rs | 33 +++++++++++++++++++---- program/tests/functional.rs | 53 ++++++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/program/src/processor.rs b/program/src/processor.rs index 0ee100f..809eef4 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -49,6 +49,33 @@ fn safe_deserialize_config_keys(input: &[u8]) -> Result Result { + let (vector_len, offset) = solana_program::short_vec::decode_shortu16_len(input) + .map_err(|_| ProgramError::InvalidAccountData)?; + if input[offset..].len() / (32 + 1) < vector_len { + Err(ProgramError::InvalidAccountData) + } else { + bincode::deserialize(input).map_err(|err| { + msg!("Unable to deserialize config account: {}", err); + ProgramError::InvalidAccountData + }) + } +} + /// Config program processor. pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult { let key_list = safe_deserialize_config_keys(input)?; @@ -60,11 +87,7 @@ pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> P return Err(ProgramError::InvalidAccountOwner); } - let current_data: ConfigKeys = bincode::deserialize(&config_account.try_borrow_data()?) - .map_err(|err| { - msg!("Unable to deserialize config account: {}", err); - ProgramError::InvalidAccountData - })?; + let current_data = safe_deserialize_config_keys_from_state(&config_account.try_borrow_data()?)?; let current_signer_keys: Vec = current_data .keys diff --git a/program/tests/functional.rs b/program/tests/functional.rs index 1631bb7..594a28f 100644 --- a/program/tests/functional.rs +++ b/program/tests/functional.rs @@ -83,7 +83,7 @@ fn test_process_create_ok() { &[(config, config_account)], &[ Check::success(), - Check::compute_units(580), + Check::compute_units(612), Check::account(&config) .data( &bincode::serialize(&(ConfigKeys { keys: vec![] }, MyConfig::default())) @@ -111,7 +111,7 @@ fn test_process_store_ok() { &[(config, config_account)], &[ Check::success(), - Check::compute_units(580), + Check::compute_units(612), Check::account(&config) .data(&bincode::serialize(&(ConfigKeys { keys }, my_config)).unwrap()) .build(), @@ -186,7 +186,7 @@ fn test_process_store_with_additional_signers() { ], &[ Check::success(), - Check::compute_units(3_234), + Check::compute_units(3_267), Check::account(&config) .data(&bincode::serialize(&(ConfigKeys { keys }, my_config)).unwrap()) .build(), @@ -334,7 +334,7 @@ fn test_config_updates() { (signer0, AccountSharedData::default()), (signer1, AccountSharedData::default()), ], - &[Check::success(), Check::compute_units(3_234)], + &[Check::success(), Check::compute_units(3_267)], ); // Use this for next invoke. @@ -352,7 +352,7 @@ fn test_config_updates() { ], &[ Check::success(), - Check::compute_units(3_235), + Check::compute_units(3_268), Check::account(&config) .data(&bincode::serialize(&(ConfigKeys { keys }, new_config)).unwrap()) .build(), @@ -465,7 +465,7 @@ fn test_config_update_contains_duplicates_fails() { (signer0, AccountSharedData::default()), (signer1, AccountSharedData::default()), ], - &[Check::success(), Check::compute_units(3_234)], + &[Check::success(), Check::compute_units(3_267)], ); // Attempt update with duplicate signer inputs. @@ -509,7 +509,7 @@ fn test_config_updates_requiring_config() { ], &[ Check::success(), - Check::compute_units(3_330), + Check::compute_units(3_363), Check::account(&config) .data(&bincode::serialize(&(ConfigKeys { keys: keys.clone() }, my_config)).unwrap()) .build(), @@ -530,7 +530,7 @@ fn test_config_updates_requiring_config() { ], &[ Check::success(), - Check::compute_units(3_330), + Check::compute_units(3_363), Check::account(&config) .data(&bincode::serialize(&(ConfigKeys { keys }, new_config)).unwrap()) .build(), @@ -624,7 +624,7 @@ fn test_maximum_keys_input() { let result = mollusk.process_and_validate_instruction( &instruction, &[(config, config_account)], - &[Check::success(), Check::compute_units(25_243)], + &[Check::success(), Check::compute_units(25_275)], ); // Use this for next invoke. @@ -637,7 +637,7 @@ fn test_maximum_keys_input() { let result = mollusk.process_and_validate_instruction( &instruction, &[(config, updated_config_account)], - &[Check::success(), Check::compute_units(25_243)], + &[Check::success(), Check::compute_units(25_275)], ); // Use this for next invoke. @@ -715,3 +715,36 @@ fn test_safe_deserialize() { &[Check::err(ProgramError::InvalidInstructionData)], ); } + +#[test] +fn test_safe_deserialize_from_state() { + let mollusk = setup(); + + let config = Pubkey::new_unique(); + + let keys = vec![(config, false), (config, true)]; + let my_config = MyConfig::new(42); + + // Input doesn't matter for this test. + let instruction = config_instruction::store(&config, false, keys.clone(), &my_config); + + // Store the keys in the config account, but give it the wrong length. + let config_account = { + let space = bincode::serialized_size(&ConfigKeys { keys: keys.clone() }).unwrap() as usize; + let lamports = mollusk.sysvars.rent.minimum_balance(space); + + let mut data = vec![0; space]; + bincode::serialize_into(&mut data, &ConfigKeys { keys }).unwrap(); + data[0] = 255; // length of 255. + + let mut account = AccountSharedData::new(lamports, space, &solana_config_program::id()); + account.set_data_from_slice(&data); + account + }; + + mollusk.process_and_validate_instruction( + &instruction, + &[(config, config_account)], + &[Check::err(ProgramError::InvalidAccountData)], + ); +} From db53895db63da460f5c7bdcd817dee650bf4768f Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Mon, 4 Nov 2024 16:41:59 +0400 Subject: [PATCH 2/2] check in new compute unit benchmark --- program/benches/compute_units.md | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/program/benches/compute_units.md b/program/benches/compute_units.md index 23e6cd8..a8abcab 100644 --- a/program/benches/compute_units.md +++ b/program/benches/compute_units.md @@ -1,3 +1,44 @@ +#### Compute Units: 2024-11-04 12:41:50.422792 UTC + +| Name | CUs | Delta | +|------|------|-------| +| config_small_init_0_keys | 612 | +32 | +| config_small_init_1_keys | 1243 | +32 | +| config_small_init_5_keys | 2862 | +32 | +| config_small_init_10_keys | 4932 | +32 | +| config_small_init_25_keys | 11774 | +32 | +| config_small_init_37_keys | 16777 | +32 | +| config_small_store_0_keys | 612 | +32 | +| config_small_store_1_keys | 1497 | +32 | +| config_small_store_5_keys | 4032 | +32 | +| config_small_store_10_keys | 7247 | +32 | +| config_small_store_25_keys | 17524 | +32 | +| config_small_store_37_keys | 25275 | +32 | +| config_medium_init_0_keys | 603 | +32 | +| config_medium_init_1_keys | 1190 | +32 | +| config_medium_init_5_keys | 2862 | +32 | +| config_medium_init_10_keys | 4932 | +32 | +| config_medium_init_25_keys | 11774 | +32 | +| config_medium_init_37_keys | 16777 | +32 | +| config_medium_store_0_keys | 603 | +32 | +| config_medium_store_1_keys | 1444 | +32 | +| config_medium_store_5_keys | 4032 | +32 | +| config_medium_store_10_keys | 7247 | +32 | +| config_medium_store_25_keys | 17524 | +32 | +| config_medium_store_37_keys | 25275 | +32 | +| config_large_init_0_keys | 724 | +32 | +| config_large_init_1_keys | 1311 | +32 | +| config_large_init_5_keys | 2983 | +32 | +| config_large_init_10_keys | 5054 | +32 | +| config_large_init_25_keys | 11898 | +32 | +| config_large_init_37_keys | 16902 | +32 | +| config_large_store_0_keys | 724 | +32 | +| config_large_store_1_keys | 1565 | +32 | +| config_large_store_5_keys | 4153 | +32 | +| config_large_store_10_keys | 7369 | +32 | +| config_large_store_25_keys | 17648 | +32 | +| config_large_store_37_keys | 25400 | +32 | + #### Compute Units: 2024-11-01 15:31:11.543055 UTC | Name | CUs | Delta |