From b88c56cd36147aaf5707e06e4983cc0cd57bba7d Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 30 Dec 2020 20:23:52 +0000 Subject: [PATCH] Limit CPI instruction size (bp #14317) (#14354) * Limit CPI instruction size (#14317) (cherry picked from commit 5524938a50bdc4c2fcc2fde52589d31a8bfbb1f1) # Conflicts: # programs/bpf_loader/src/syscalls.rs # sdk/src/feature_set.rs * resolve conflicts Co-authored-by: Jack May --- programs/bpf/c/src/invoke/invoke.c | 32 ++++++++++++++ programs/bpf/rust/invoke/src/lib.rs | 17 ++++++++ programs/bpf/tests/programs.rs | 66 +++++++++++++++++++++++++++++ programs/bpf_loader/src/lib.rs | 1 + programs/bpf_loader/src/syscalls.rs | 33 +++++++++++++-- sdk/src/feature_set.rs | 5 +++ sdk/src/process_instruction.rs | 13 +++++- 7 files changed, 162 insertions(+), 5 deletions(-) diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index 11d5b90a2a..905886a50b 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -12,6 +12,8 @@ static const uint8_t TEST_EMPTY_ACCOUNTS_SLICE = 5; static const uint8_t TEST_CAP_SEEDS = 6; static const uint8_t TEST_CAP_SIGNERS = 7; static const uint8_t TEST_ALLOC_ACCESS_VIOLATION = 8; +static const uint8_t TEST_INSTRUCTION_DATA_TOO_LARGE = 9; +static const uint8_t TEST_INSTRUCTION_META_TOO_LARGE = 10; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -390,6 +392,36 @@ extern uint64_t entrypoint(const uint8_t *input) { signers_seeds, SOL_ARRAY_SIZE(signers_seeds))); break; } + case TEST_INSTRUCTION_DATA_TOO_LARGE: { + sol_log("Test instruction data too large"); + SolAccountMeta arguments[] = {}; + uint8_t *data = sol_calloc(1500, 1); + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, 1500}; + const SolSignerSeeds signers_seeds[] = {}; + sol_assert(SUCCESS == sol_invoke_signed( + &instruction, accounts, SOL_ARRAY_SIZE(accounts), + signers_seeds, SOL_ARRAY_SIZE(signers_seeds))); + + break; + } + case TEST_INSTRUCTION_META_TOO_LARGE: { + sol_log("Test instruction meta too large"); + SolAccountMeta *arguments = sol_calloc(40, sizeof(SolAccountMeta)); + sol_log_64(0, 0, 0, 0, (uint64_t)arguments); + sol_assert(0 != arguments); + uint8_t data[] = {}; + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, 40, data, + SOL_ARRAY_SIZE(data)}; + const SolSignerSeeds signers_seeds[] = {}; + sol_assert(SUCCESS == sol_invoke_signed( + &instruction, accounts, SOL_ARRAY_SIZE(accounts), + signers_seeds, SOL_ARRAY_SIZE(signers_seeds))); + + break; + } default: sol_panic(); } diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index 9b8ccfc549..25757371a6 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -24,6 +24,8 @@ const TEST_EMPTY_ACCOUNTS_SLICE: u8 = 5; const TEST_CAP_SEEDS: u8 = 6; const TEST_CAP_SIGNERS: u8 = 7; const TEST_ALLOC_ACCESS_VIOLATION: u8 = 8; +const TEST_INSTRUCTION_DATA_TOO_LARGE: u8 = 9; +const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10; // const MINT_INDEX: usize = 0; const ARGUMENT_INDEX: usize = 1; @@ -470,6 +472,21 @@ fn process_instruction( &[&[b"You pass butter", &[bump_seed1]]], )?; } + TEST_INSTRUCTION_DATA_TOO_LARGE => { + msg!("Test instruction data too large"); + let instruction = + create_instruction(*accounts[INVOKED_PROGRAM_INDEX].key, &[], vec![0; 1500]); + invoke_signed(&instruction, &[], &[])?; + } + TEST_INSTRUCTION_META_TOO_LARGE => { + msg!("Test instruction metas too large"); + let instruction = create_instruction( + *accounts[INVOKED_PROGRAM_INDEX].key, + &[(&Pubkey::default(), false, false); 40], + vec![], + ); + invoke_signed(&instruction, &[], &[])?; + } _ => panic!(), } diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 9277fa28f6..9717e955c6 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -521,6 +521,8 @@ fn test_program_bpf_invoke() { const TEST_CAP_SEEDS: u8 = 6; const TEST_CAP_SIGNERS: u8 = 7; const TEST_ALLOC_ACCESS_VIOLATION: u8 = 8; + const TEST_INSTRUCTION_DATA_TOO_LARGE: u8 = 9; + const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10; #[allow(dead_code)] #[derive(Debug)] @@ -849,6 +851,70 @@ fn test_program_bpf_invoke() { TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) ); + let instruction = Instruction::new( + invoke_program_id, + &[ + TEST_INSTRUCTION_DATA_TOO_LARGE, + bump_seed1, + bump_seed2, + bump_seed3, + ], + account_metas.clone(), + ); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let tx = Transaction::new( + &[ + &mint_keypair, + &argument_keypair, + &invoked_argument_keypair, + &from_keypair, + ], + message.clone(), + bank.last_blockhash(), + ); + let (result, inner_instructions) = process_transaction_and_record_inner(&bank, tx); + let invoked_programs: Vec = inner_instructions[0] + .iter() + .map(|ix| message.account_keys[ix.program_id_index as usize].clone()) + .collect(); + assert_eq!(invoked_programs, vec![]); + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError(0, InstructionError::ComputationalBudgetExceeded) + ); + + let instruction = Instruction::new( + invoke_program_id, + &[ + TEST_INSTRUCTION_META_TOO_LARGE, + bump_seed1, + bump_seed2, + bump_seed3, + ], + account_metas.clone(), + ); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let tx = Transaction::new( + &[ + &mint_keypair, + &argument_keypair, + &invoked_argument_keypair, + &from_keypair, + ], + message.clone(), + bank.last_blockhash(), + ); + let (result, inner_instructions) = process_transaction_and_record_inner(&bank, tx); + let invoked_programs: Vec = inner_instructions[0] + .iter() + .map(|ix| message.account_keys[ix.program_id_index as usize].clone()) + .collect(); + assert_eq!(invoked_programs, vec![]); + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError(0, InstructionError::ComputationalBudgetExceeded) + ); + // Check final state assert_eq!(43, bank.get_balance(&derived_key1)); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 2a98e3dce3..5f9b2eb57d 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -935,6 +935,7 @@ mod tests { max_call_depth: 20, stack_frame_size: 4096, log_pubkey_units: 100, + max_cpi_instruction_size: usize::MAX, }, Rc::new(RefCell::new(Executors::default())), None, diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 43626f503b..9c759536f2 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -652,6 +652,7 @@ trait SyscallInvokeSigned<'a> { fn translate_instruction( &self, addr: u64, + max_size: usize, ro_regions: &[MemoryRegion], ) -> Result>; fn translate_accounts( @@ -689,9 +690,12 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { fn translate_instruction( &self, addr: u64, + max_size: usize, ro_regions: &[MemoryRegion], ) -> Result> { let ix = translate_type!(Instruction, addr, ro_regions, self.loader_id)?; + check_instruction_size(ix.accounts.len(), ix.data.len(), max_size)?; + let accounts = translate_slice!( AccountMeta, ix.accounts.as_ptr(), @@ -960,15 +964,19 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { .try_borrow_mut() .map_err(|_| SyscallError::InvokeContextBorrowFailed.into()) } + fn get_callers_keyed_accounts(&self) -> &'a [KeyedAccount<'a>] { self.callers_keyed_accounts } + fn translate_instruction( &self, addr: u64, + max_size: usize, ro_regions: &[MemoryRegion], ) -> Result> { let ix_c = translate_type!(SolInstruction, addr, ro_regions, self.loader_id)?; + check_instruction_size(ix_c.accounts_len, ix_c.data_len, max_size)?; let program_id = translate_type!(Pubkey, ix_c.program_id_addr, ro_regions, self.loader_id)?; let meta_cs = translate_slice!( SolAccountMeta, @@ -1161,7 +1169,20 @@ impl<'a> SyscallObject for SyscallInvokeSignedC<'a> { } } -fn is_authorized_program(program_id: &Pubkey) -> Result<(), EbpfError> { +fn check_instruction_size( + num_accounts: usize, + data_len: usize, + max_size: usize, +) -> Result<(), EbpfError> { + if max_size < num_accounts * size_of::() + data_len { + return Err( + SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded).into(), + ); + } + Ok(()) +} + +fn check_authorized_program(program_id: &Pubkey) -> Result<(), EbpfError> { if native_loader::check_id(program_id) || bpf_loader::check_id(program_id) || bpf_loader_deprecated::check_id(program_id) @@ -1190,7 +1211,13 @@ fn call<'a>( // Translate and verify caller's data - let instruction = syscall.translate_instruction(instruction_addr, ro_regions)?; + let instruction = syscall.translate_instruction( + instruction_addr, + invoke_context + .get_bpf_compute_budget() + .max_cpi_instruction_size, + ro_regions, + )?; let caller_program_id = invoke_context .get_caller() .map_err(SyscallError::InstructionError)?; @@ -1207,7 +1234,7 @@ fn call<'a>( let (message, callee_program_id) = MessageProcessor::create_message(&instruction, &keyed_account_refs, &signers) .map_err(SyscallError::InstructionError)?; - is_authorized_program(&callee_program_id)?; + check_authorized_program(&callee_program_id)?; let (accounts, account_refs) = syscall.translate_accounts( &message, account_infos_addr, diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 716c76034e..0ab99b2984 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -110,6 +110,10 @@ pub mod bpf_loader_upgradeable_program { solana_sdk::declare_id!("FbhK8HN9qvNHvJcoFVHAEUCNkagHvu7DTWzdnLuVQ5u4"); } +pub mod max_cpi_instruction_size_ipv6_mtu { + solana_sdk::declare_id!("5WLtuUJA5VVA1Cc28qULPfGs8anhoBev8uNqaaXeasnf"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -139,6 +143,7 @@ lazy_static! { (simple_capitalization::id(), "simple capitalization"), (stake_program_v3::id(), "solana_stake_program v3"), (bpf_loader_upgradeable_program::id(), "upgradeable bpf loader"), + (max_cpi_instruction_size_ipv6_mtu::id(), "Max cross-program invocation size 1280"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/process_instruction.rs b/sdk/src/process_instruction.rs index 26f3477e7e..20922d514d 100644 --- a/sdk/src/process_instruction.rs +++ b/sdk/src/process_instruction.rs @@ -1,8 +1,8 @@ use solana_sdk::{ account::Account, feature_set::{ - bpf_compute_budget_balancing, max_invoke_depth_4, max_program_call_depth_64, - pubkey_log_syscall_enabled, FeatureSet, + bpf_compute_budget_balancing, max_cpi_instruction_size_ipv6_mtu, max_invoke_depth_4, + max_program_call_depth_64, pubkey_log_syscall_enabled, FeatureSet, }, instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::KeyedAccount, @@ -91,6 +91,8 @@ pub struct BpfComputeBudget { pub stack_frame_size: usize, /// Number of compute units consumed by logging a `Pubkey` pub log_pubkey_units: u64, + /// Maximum cross-program invocation instruction size + pub max_cpi_instruction_size: usize, } impl Default for BpfComputeBudget { fn default() -> Self { @@ -113,6 +115,7 @@ impl BpfComputeBudget { max_call_depth: 20, stack_frame_size: 4_096, log_pubkey_units: 0, + max_cpi_instruction_size: std::usize::MAX, }; if feature_set.is_active(&bpf_compute_budget_balancing::id()) { @@ -144,6 +147,12 @@ impl BpfComputeBudget { ..bpf_compute_budget }; } + if feature_set.is_active(&max_cpi_instruction_size_ipv6_mtu::id()) { + bpf_compute_budget = BpfComputeBudget { + max_cpi_instruction_size: 1280, // IPv6 Min MTU size + ..bpf_compute_budget + }; + } bpf_compute_budget } }