From 025f886e101d49d42e7769df8a6eada902af6927 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 15 Dec 2020 23:21:08 -0800 Subject: [PATCH] check for resize access violations (#14142) --- programs/bpf/c/src/invoke/invoke.c | 43 ++++++++++++++++++ programs/bpf/rust/invoke/src/lib.rs | 69 ++++++++++++++++++++++++++++- programs/bpf/tests/programs.rs | 37 ++++++++++++++++ programs/bpf_loader/src/syscalls.rs | 22 ++++++--- 4 files changed, 165 insertions(+), 6 deletions(-) diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index 5d0188b1df..c36dac2a17 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -11,6 +11,7 @@ static const uint8_t TEST_PPROGRAM_NOT_EXECUTABLE = 4; 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 int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -297,6 +298,7 @@ extern uint64_t entrypoint(const uint8_t *input) { data, SOL_ARRAY_SIZE(data)}; sol_assert(SUCCESS == sol_invoke(&instruction, 0, 0)); + break; } case TEST_CAP_SEEDS: { sol_log("Test cap seeds"); @@ -321,6 +323,7 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(SUCCESS == sol_invoke_signed( &instruction, accounts, SOL_ARRAY_SIZE(accounts), signers_seeds, SOL_ARRAY_SIZE(signers_seeds))); + break; } case TEST_CAP_SIGNERS: { sol_log("Test cap signers"); @@ -360,6 +363,46 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(SUCCESS == sol_invoke_signed( &instruction, accounts, SOL_ARRAY_SIZE(accounts), signers_seeds, SOL_ARRAY_SIZE(signers_seeds))); + break; + } + case TEST_ALLOC_ACCESS_VIOLATION: { + sol_log("Test resize violation"); + SolAccountMeta arguments[] = { + {accounts[FROM_INDEX].key, true, true}, + {accounts[DERIVED_KEY1_INDEX].key, true, true}}; + uint8_t data[4 + 8 + 8 + 32]; + *(uint64_t *)(data + 4) = 42; + *(uint64_t *)(data + 4 + 8) = MAX_PERMITTED_DATA_INCREASE; + sol_memcpy(data + 4 + 8 + 8, params.program_id, SIZE_PUBKEY); + const SolInstruction instruction = {accounts[SYSTEM_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + uint8_t seed1[] = {'Y', 'o', 'u', ' ', 'p', 'a', 's', 's', + ' ', 'b', 'u', 't', 't', 'e', 'r'}; + const SolSignerSeed seeds1[] = {{seed1, SOL_ARRAY_SIZE(seed1)}, + {&bump_seed1, 1}}; + const SolSignerSeeds signers_seeds[] = {{seeds1, SOL_ARRAY_SIZE(seeds1)}}; + + SolAccountInfo derived_account = { + .key = accounts[DERIVED_KEY1_INDEX].key, + .lamports = accounts[DERIVED_KEY1_INDEX].lamports, + .data_len = accounts[DERIVED_KEY1_INDEX].data_len, + // Point to top edge of heap, attempt to allocate into unprivileged + // memory + .data = (uint8_t *)0x300007ff8, + .owner = accounts[DERIVED_KEY1_INDEX].owner, + .rent_epoch = accounts[DERIVED_KEY1_INDEX].rent_epoch, + .is_signer = accounts[DERIVED_KEY1_INDEX].is_signer, + .is_writable = accounts[DERIVED_KEY1_INDEX].is_writable, + .executable = accounts[DERIVED_KEY1_INDEX].executable, + }; + const SolAccountInfo invoke_accounts[] = { + accounts[FROM_INDEX], accounts[SYSTEM_PROGRAM_INDEX], derived_account}; + sol_assert(SUCCESS == + sol_invoke_signed(&instruction, + (const SolAccountInfo *)invoke_accounts, 3, + 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 2bb423f185..9c33c5ef09 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -23,6 +23,7 @@ const TEST_PPROGRAM_NOT_EXECUTABLE: u8 = 4; 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 MINT_INDEX: usize = 0; const ARGUMENT_INDEX: usize = 1; @@ -33,7 +34,7 @@ const INVOKED_PROGRAM_DUP_INDEX: usize = 4; const DERIVED_KEY1_INDEX: usize = 6; const DERIVED_KEY2_INDEX: usize = 7; const DERIVED_KEY3_INDEX: usize = 8; -// const SYSTEM_PROGRAM_INDEX: usize = 9; +const SYSTEM_PROGRAM_INDEX: usize = 9; const FROM_INDEX: usize = 10; entrypoint!(process_instruction); @@ -426,6 +427,72 @@ fn process_instruction( ], )?; } + TEST_ALLOC_ACCESS_VIOLATION => { + msg!("Test resize violation"); + let pubkey = *accounts[FROM_INDEX].key; + let owner = *accounts[FROM_INDEX].owner; + let ptr = accounts[FROM_INDEX].data.borrow().as_ptr() as u64 as *mut _; + let len = accounts[FROM_INDEX].data_len(); + let mut data = unsafe { std::slice::from_raw_parts_mut(ptr, len) }; + let mut lamports = accounts[FROM_INDEX].lamports(); + let from_info = AccountInfo::new( + &pubkey, + false, + true, + &mut lamports, + &mut data, + &owner, + false, + 0, + ); + + let pubkey = *accounts[DERIVED_KEY1_INDEX].key; + let owner = *accounts[DERIVED_KEY1_INDEX].owner; + // Point to top edge of heap, attempt to allocate into unprivileged memory + let mut data = unsafe { std::slice::from_raw_parts_mut(0x300007ff8 as *mut _, 0) }; + let mut lamports = accounts[DERIVED_KEY1_INDEX].lamports(); + let derived_info = AccountInfo::new( + &pubkey, + false, + true, + &mut lamports, + &mut data, + &owner, + false, + 0, + ); + + let pubkey = *accounts[SYSTEM_PROGRAM_INDEX].key; + let owner = *accounts[SYSTEM_PROGRAM_INDEX].owner; + let ptr = accounts[SYSTEM_PROGRAM_INDEX].data.borrow().as_ptr() as u64 as *mut _; + let len = accounts[SYSTEM_PROGRAM_INDEX].data_len(); + let mut data = unsafe { std::slice::from_raw_parts_mut(ptr, len) }; + let mut lamports = accounts[SYSTEM_PROGRAM_INDEX].lamports(); + let system_info = AccountInfo::new( + &pubkey, + false, + false, + &mut lamports, + &mut data, + &owner, + true, + 0, + ); + + let instruction = system_instruction::create_account( + accounts[FROM_INDEX].key, + accounts[DERIVED_KEY1_INDEX].key, + 42, + MAX_PERMITTED_DATA_INCREASE as u64, + program_id, + ); + + invoke_signed( + &instruction, + &[system_info.clone(), from_info.clone(), derived_info.clone()], + &[&[b"You pass butter", &[bump_seed1]]], + )?; + } _ => panic!(), } diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 934711b789..acc72dce54 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -565,6 +565,7 @@ fn test_program_bpf_invoke() { 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; #[allow(dead_code)] #[derive(Debug)] @@ -905,6 +906,42 @@ fn test_program_bpf_invoke() { for i in 0..20 { assert_eq!(i as u8, account.data[i]); } + + // Attempt to realloc into unauthorized address space + let account = Account::new(84, 0, &solana_sdk::system_program::id()); + bank.store_account(&from_keypair.pubkey(), &account); + bank.store_account(&derived_key1, &Account::default()); + let instruction = Instruction::new( + invoke_program_id, + &[ + TEST_ALLOC_ACCESS_VIOLATION, + 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![solana_sdk::system_program::id()]); + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) + ); } // Check for program id spoofing diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index dc81bc9bd7..ee19cc56e6 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -827,6 +827,7 @@ struct AccountReferences<'a> { lamports: &'a mut u64, owner: &'a mut Pubkey, data: &'a mut [u8], + vm_data_addr: u64, ref_to_len_in_vm: &'a mut u64, serialized_len_ptr: &'a mut u64, } @@ -941,7 +942,7 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { account_info.owner as *const _ as u64, self.loader_id, )?; - let (data, ref_to_len_in_vm, serialized_len_ptr) = { + let (data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr) = { // Double translate data out of RefCell let data = *translate_type::<&[u8]>( memory_mapping, @@ -961,13 +962,15 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { ref_of_len_in_input_buffer as *const _ as u64, self.loader_id, )?; + let vm_data_addr = data.as_ptr() as u64; ( translate_slice_mut::( memory_mapping, - data.as_ptr() as u64, + vm_data_addr, data.len() as u64, self.loader_id, )?, + vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, ) @@ -984,6 +987,7 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { lamports, owner, data, + vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, }); @@ -1206,9 +1210,10 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { account_info.owner_addr, self.loader_id, )?; + let vm_data_addr = account_info.data_addr; let data = translate_slice_mut::( memory_mapping, - account_info.data_addr, + vm_data_addr, account_info.data_len, self.loader_id, )?; @@ -1243,6 +1248,7 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { lamports, owner, data, + vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, }); @@ -1408,8 +1414,6 @@ fn call<'a>( *account_ref.lamports = account.lamports; *account_ref.owner = account.owner; if account_ref.data.len() != account.data.len() { - *account_ref.ref_to_len_in_vm = account.data.len() as u64; - *account_ref.serialized_len_ptr = account.data.len() as u64; if !account_ref.data.is_empty() { // Only support for `CreateAccount` at this time. // Need a way to limit total realloc size across multiple CPI calls @@ -1422,6 +1426,14 @@ fn call<'a>( SyscallError::InstructionError(InstructionError::InvalidRealloc).into(), ); } + let _ = translate( + memory_mapping, + AccessType::Store, + account_ref.vm_data_addr, + account.data.len() as u64, + )?; + *account_ref.ref_to_len_in_vm = account.data.len() as u64; + *account_ref.serialized_len_ptr = account.data.len() as u64; } account_ref .data