diff --git a/programs/bpf_loader_api/src/lib.rs b/programs/bpf_loader_api/src/lib.rs index 029933aded..2bd4fe04dd 100644 --- a/programs/bpf_loader_api/src/lib.rs +++ b/programs/bpf_loader_api/src/lib.rs @@ -18,7 +18,7 @@ use log::*; use solana_rbpf::{memory_region::MemoryRegion, EbpfVm}; use solana_sdk::account::KeyedAccount; use solana_sdk::instruction::InstructionError; -use solana_sdk::instruction_processor_utils::limited_deserialize; +use solana_sdk::instruction_processor_utils::{limited_deserialize, next_keyed_account}; use solana_sdk::loader_instruction::LoaderInstruction; use solana_sdk::pubkey::Pubkey; use solana_sdk::sysvar::rent; @@ -93,58 +93,61 @@ pub fn process_instruction( if let Ok(instruction) = limited_deserialize(ix_data) { match instruction { LoaderInstruction::Write { offset, bytes } => { - if keyed_accounts[0].signer_key().is_none() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + if program.signer_key().is_none() { warn!("key[0] did not sign the transaction"); - return Err(InstructionError::GenericError); + return Err(InstructionError::MissingRequiredSignature); } let offset = offset as usize; let len = bytes.len(); trace!("Write: offset={} length={}", offset, len); - if keyed_accounts[0].account.data.len() < offset + len { + if program.account.data.len() < offset + len { warn!( "Write overflow: {} < {}", - keyed_accounts[0].account.data.len(), + program.account.data.len(), offset + len ); - return Err(InstructionError::GenericError); + return Err(InstructionError::AccountDataTooSmall); } - keyed_accounts[0].account.data[offset..offset + len].copy_from_slice(&bytes); + program.account.data[offset..offset + len].copy_from_slice(&bytes); } LoaderInstruction::Finalize => { - if keyed_accounts.len() < 2 { - return Err(InstructionError::InvalidInstructionData); - } - if keyed_accounts[0].signer_key().is_none() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + let rent = next_keyed_account(&mut keyed_accounts_iter)?; + + if program.signer_key().is_none() { warn!("key[0] did not sign the transaction"); - return Err(InstructionError::GenericError); + return Err(InstructionError::MissingRequiredSignature); } - rent::verify_rent_exemption(&keyed_accounts[0], &keyed_accounts[1])?; + rent::verify_rent_exemption(&program, &rent)?; - keyed_accounts[0].account.executable = true; - info!( - "Finalize: account {:?}", - keyed_accounts[0].signer_key().unwrap() - ); + program.account.executable = true; + info!("Finalize: account {:?}", program.signer_key().unwrap()); } LoaderInstruction::InvokeMain { data } => { - if !keyed_accounts[0].account.executable { - warn!("BPF account not executable"); - return Err(InstructionError::GenericError); + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + + if !program.account.executable { + warn!("BPF program account not executable"); + return Err(InstructionError::AccountNotExecutable); } - let (progs, params) = keyed_accounts.split_at_mut(1); - let prog = &progs[0].account.data; - let (mut vm, heap_region) = match create_vm(prog) { + let (mut vm, heap_region) = match create_vm(&program.account.data) { Ok(info) => info, Err(e) => { warn!("Failed to create BPF VM: {}", e); return Err(InstructionError::GenericError); } }; - let mut v = serialize_parameters(program_id, params, &data); + let parameter_accounts = keyed_accounts_iter.into_slice(); + let mut parameter_bytes = + serialize_parameters(program_id, parameter_accounts, &data); info!("Call BPF program"); - match vm.execute_program(v.as_mut_slice(), &[], &[heap_region]) { + match vm.execute_program(parameter_bytes.as_mut_slice(), &[], &[heap_region]) { Ok(status) => match u32::try_from(status) { Ok(status) => { if status > 0 { @@ -162,13 +165,13 @@ pub fn process_instruction( return Err(InstructionError::GenericError); } } - deserialize_parameters(params, &v); + deserialize_parameters(parameter_accounts, ¶meter_bytes); info!("BPF program success"); } } } else { warn!("Invalid instruction data: {:?}", ix_data); - return Err(InstructionError::GenericError); + return Err(InstructionError::InvalidInstructionData); } Ok(()) } @@ -176,12 +179,15 @@ pub fn process_instruction( #[cfg(test)] mod tests { use super::*; + use solana_sdk::account::Account; + use std::fs::File; + use std::io::Read; #[test] #[should_panic(expected = "Error: Exceeded maximum number of instructions allowed")] - fn test_non_terminating_program() { + fn test_bpf_loader_non_terminating_program() { #[rustfmt::skip] - let prog = &[ + let program = &[ 0x07, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, // r6 + 1 0x05, 0x00, 0xfe, 0xff, 0x00, 0x00, 0x00, 0x00, // goto -2 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit @@ -191,7 +197,134 @@ mod tests { let mut vm = EbpfVm::new(None).unwrap(); vm.set_verifier(bpf_verifier::check).unwrap(); vm.set_max_instruction_count(10).unwrap(); - vm.set_program(prog).unwrap(); + vm.set_program(program).unwrap(); vm.execute_program(input, &[], &[]).unwrap(); } + + #[test] + fn test_bpf_loader_write() { + let program_id = Pubkey::new_rand(); + let program_key = Pubkey::new_rand(); + let mut program_account = Account::new(1, 0, &program_id); + let mut keyed_accounts = vec![KeyedAccount::new(&program_key, false, &mut program_account)]; + let ix_data = bincode::serialize(&LoaderInstruction::Write { + offset: 3, + bytes: vec![1, 2, 3], + }) + .unwrap(); + + // Case: Empty keyed accounts + assert_eq!( + Err(InstructionError::NotEnoughAccountKeys), + process_instruction(&program_id, &mut vec![], &ix_data) + ); + + // Case: Not signed + assert_eq!( + Err(InstructionError::MissingRequiredSignature), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + + // Case: Write bytes to an offset + let mut keyed_accounts = vec![KeyedAccount::new(&program_key, true, &mut program_account)]; + keyed_accounts[0].account.data = vec![0; 6]; + assert_eq!( + Ok(()), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + assert_eq!(vec![0, 0, 0, 1, 2, 3], keyed_accounts[0].account.data); + + // Case: Overflow + let mut keyed_accounts = vec![KeyedAccount::new(&program_key, true, &mut program_account)]; + keyed_accounts[0].account.data = vec![0; 5]; + assert_eq!( + Err(InstructionError::AccountDataTooSmall), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + } + + #[test] + fn test_bpf_loader_finalize() { + let program_id = Pubkey::new_rand(); + let program_key = Pubkey::new_rand(); + let rent_key = rent::id(); + let mut program_account = Account::new(1, 0, &program_id); + let mut keyed_accounts = vec![KeyedAccount::new(&program_key, false, &mut program_account)]; + let ix_data = bincode::serialize(&LoaderInstruction::Finalize).unwrap(); + + // Case: Empty keyed accounts + assert_eq!( + Err(InstructionError::NotEnoughAccountKeys), + process_instruction(&program_id, &mut vec![], &ix_data) + ); + + let mut rent_account = rent::create_account(1, &rent::Rent::default()); + keyed_accounts.push(KeyedAccount::new(&rent_key, false, &mut rent_account)); + + // Case: Not signed + assert_eq!( + Err(InstructionError::MissingRequiredSignature), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + + // Case: Finalize + let mut keyed_accounts = vec![ + KeyedAccount::new(&program_key, true, &mut program_account), + KeyedAccount::new(&rent_key, false, &mut rent_account), + ]; + assert_eq!( + Ok(()), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + assert!(keyed_accounts[0].account.executable); + } + + #[test] + fn test_bpf_loader_invoke_main() { + let program_id = Pubkey::new_rand(); + let program_key = Pubkey::new_rand(); + + // Create program account + let mut file = File::open("test_elfs/noop.so").expect("file open failed"); + let mut elf = Vec::new(); + file.read_to_end(&mut elf).unwrap(); + let mut program_account = Account::new(1, 0, &program_id); + program_account.data = elf; + program_account.executable = true; + + let mut keyed_accounts = vec![KeyedAccount::new(&program_key, false, &mut program_account)]; + let ix_data = bincode::serialize(&LoaderInstruction::InvokeMain { data: vec![] }).unwrap(); + + // Case: Empty keyed accounts + assert_eq!( + Err(InstructionError::NotEnoughAccountKeys), + process_instruction(&program_id, &mut vec![], &ix_data) + ); + + // Case: Only a program account + assert_eq!( + Ok(()), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + + // Case: Account not executable + keyed_accounts[0].account.executable = false; + assert_eq!( + Err(InstructionError::AccountNotExecutable), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + keyed_accounts[0].account.executable = true; + + // Case: With program and parameter account + let mut parameter_account = Account::new(1, 0, &program_id); + keyed_accounts.push(KeyedAccount::new( + &program_key, + false, + &mut parameter_account, + )); + assert_eq!( + Ok(()), + process_instruction(&program_id, &mut keyed_accounts, &ix_data) + ); + } } diff --git a/programs/bpf_loader_api/test_elfs/noop.so b/programs/bpf_loader_api/test_elfs/noop.so new file mode 100755 index 0000000000..1259bfa04a Binary files /dev/null and b/programs/bpf_loader_api/test_elfs/noop.so differ diff --git a/programs/move_loader_api/src/processor.rs b/programs/move_loader_api/src/processor.rs index 8cdfb688c9..dc9d519a27 100644 --- a/programs/move_loader_api/src/processor.rs +++ b/programs/move_loader_api/src/processor.rs @@ -5,9 +5,13 @@ use bytecode_verifier::verifier::{VerifiedModule, VerifiedScript}; use log::*; use serde_derive::{Deserialize, Serialize}; use solana_sdk::{ - account::KeyedAccount, instruction::InstructionError, - instruction_processor_utils::limited_deserialize, loader_instruction::LoaderInstruction, - move_loader::id, pubkey::Pubkey, sysvar::rent, + account::KeyedAccount, + instruction::InstructionError, + instruction_processor_utils::{limited_deserialize, next_keyed_account}, + loader_instruction::LoaderInstruction, + move_loader::id, + pubkey::Pubkey, + sysvar::rent, }; use types::{ account_address::AccountAddress, @@ -50,9 +54,6 @@ pub fn process_instruction( } } -pub const PROGRAM_INDEX: usize = 0; -pub const GENESIS_INDEX: usize = 1; - /// Command to invoke #[derive(Debug, Serialize, Deserialize)] pub enum InvokeCommand { @@ -228,21 +229,30 @@ impl MoveProcessor { } fn keyed_accounts_to_data_store( - genesis_key: &Pubkey, keyed_accounts: &[KeyedAccount], ) -> Result { let mut data_store = DataStore::default(); - for keyed_account in keyed_accounts { - match limited_deserialize(&keyed_account.account.data)? { - LibraAccountState::Genesis(write_set) => data_store.apply_write_set(&write_set), - LibraAccountState::User(owner, write_set) => { - if owner != *genesis_key { - debug!("All user accounts must be owned by the genesis"); - return Err(InstructionError::InvalidArgument); - } - data_store.apply_write_set(&write_set) + + let mut keyed_accounts_iter = keyed_accounts.iter(); + let genesis = next_keyed_account(&mut keyed_accounts_iter)?; + + match limited_deserialize(&genesis.account.data)? { + LibraAccountState::Genesis(write_set) => data_store.apply_write_set(&write_set), + _ => { + debug!("Must include a genesis account"); + return Err(InstructionError::InvalidArgument); + } + } + + for keyed_account in keyed_accounts_iter { + if let LibraAccountState::User(owner, write_set) = + limited_deserialize(&keyed_account.account.data)? + { + if owner != *genesis.unsigned_key() { + debug!("All user accounts must be owned by the genesis"); + return Err(InstructionError::InvalidArgument); } - _ => (), // ignore unallocated accounts + data_store.apply_write_set(&write_set) } } Ok(data_store) @@ -256,8 +266,11 @@ impl MoveProcessor { .into_write_sets() .map_err(|_| InstructionError::GenericError)?; + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + // Genesis account holds both mint and stdlib - let genesis_key = *keyed_accounts[GENESIS_INDEX].unsigned_key(); + let genesis = next_keyed_account(&mut keyed_accounts_iter)?; + let genesis_key = *genesis.unsigned_key(); let mut write_set = write_sets .remove(&account_config::association_address()) .ok_or_else(Self::missing_account)? @@ -272,11 +285,11 @@ impl MoveProcessor { let write_set = write_set.freeze().unwrap(); Self::serialize_and_enforce_length( &LibraAccountState::Genesis(write_set), - &mut keyed_accounts[GENESIS_INDEX].account.data, + &mut genesis.account.data, )?; // Now do the rest of the accounts - for keyed_account in keyed_accounts[GENESIS_INDEX + 1..].iter_mut() { + for keyed_account in keyed_accounts_iter { let write_set = write_sets .remove(&pubkey_to_address(keyed_account.unsigned_key())) .ok_or_else(Self::missing_account)?; @@ -297,38 +310,42 @@ impl MoveProcessor { offset: u32, bytes: &[u8], ) -> Result<(), InstructionError> { - if keyed_accounts[PROGRAM_INDEX].signer_key().is_none() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + + if program.signer_key().is_none() { debug!("Error: key[0] did not sign the transaction"); - return Err(InstructionError::GenericError); + return Err(InstructionError::MissingRequiredSignature); } let offset = offset as usize; let len = bytes.len(); trace!("Write: offset={} length={}", offset, len); - if keyed_accounts[PROGRAM_INDEX].account.data.len() < offset + len { + if program.account.data.len() < offset + len { debug!( "Error: Write overflow: {} < {}", - keyed_accounts[PROGRAM_INDEX].account.data.len(), + program.account.data.len(), offset + len ); - return Err(InstructionError::GenericError); + return Err(InstructionError::AccountDataTooSmall); } - keyed_accounts[PROGRAM_INDEX].account.data[offset..offset + len].copy_from_slice(&bytes); + program.account.data[offset..offset + len].copy_from_slice(&bytes); Ok(()) } pub fn do_finalize(keyed_accounts: &mut [KeyedAccount]) -> Result<(), InstructionError> { - if keyed_accounts.len() < 2 { - return Err(InstructionError::InvalidInstructionData); - } - if keyed_accounts[PROGRAM_INDEX].signer_key().is_none() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + let rent = next_keyed_account(&mut keyed_accounts_iter)?; + + if program.signer_key().is_none() { debug!("Error: key[0] did not sign the transaction"); - return Err(InstructionError::GenericError); + return Err(InstructionError::MissingRequiredSignature); } - rent::verify_rent_exemption(&keyed_accounts[0], &keyed_accounts[1])?; + rent::verify_rent_exemption(&program, &rent)?; let (compiled_script, compiled_modules) = - Self::deserialize_compiled_program(&keyed_accounts[PROGRAM_INDEX].account.data)?; + Self::deserialize_compiled_program(&program.account.data)?; let verified_script = VerifiedScript::new(compiled_script).unwrap(); let verified_modules = compiled_modules @@ -339,16 +356,14 @@ impl MoveProcessor { Self::serialize_and_enforce_length( &Self::verify_program(&verified_script, &verified_modules)?, - &mut keyed_accounts[PROGRAM_INDEX].account.data, + &mut program.account.data, )?; - keyed_accounts[PROGRAM_INDEX].account.executable = true; + program.account.executable = true; info!( "Finalize: {:?}", - keyed_accounts[PROGRAM_INDEX] - .signer_key() - .unwrap_or(&Pubkey::default()) + program.signer_key().unwrap_or(&Pubkey::default()) ); Ok(()) } @@ -359,19 +374,18 @@ impl MoveProcessor { ) -> Result<(), InstructionError> { match limited_deserialize(&data)? { InvokeCommand::CreateGenesis(amount) => { - if keyed_accounts.is_empty() { - debug!("Error: Requires an unallocated account"); - return Err(InstructionError::InvalidArgument); - } - if keyed_accounts[0].account.owner != id() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + + if program.account.owner != id() { debug!("Error: Move program account not owned by Move loader"); return Err(InstructionError::InvalidArgument); } - match limited_deserialize(&keyed_accounts[0].account.data)? { + match limited_deserialize(&program.account.data)? { LibraAccountState::Unallocated => Self::serialize_and_enforce_length( &LibraAccountState::create_genesis(amount)?, - &mut keyed_accounts[0].account.data, + &mut program.account.data, ), _ => { debug!("Error: Must provide an unallocated account"); @@ -384,26 +398,23 @@ impl MoveProcessor { function_name, args, } => { - if keyed_accounts.len() < 2 { - debug!("Error: Requires at least a program and a genesis accounts"); - return Err(InstructionError::InvalidArgument); - } - if keyed_accounts[PROGRAM_INDEX].account.owner != id() { + let mut keyed_accounts_iter = keyed_accounts.iter_mut(); + let program = next_keyed_account(&mut keyed_accounts_iter)?; + + if program.account.owner != id() { debug!("Error: Move program account not owned by Move loader"); return Err(InstructionError::InvalidArgument); } - if !keyed_accounts[PROGRAM_INDEX].account.executable { + if !program.account.executable { debug!("Error: Move program account not executable"); - return Err(InstructionError::InvalidArgument); + return Err(InstructionError::AccountNotExecutable); } - let mut data_store = Self::keyed_accounts_to_data_store( - keyed_accounts[GENESIS_INDEX].unsigned_key(), - &keyed_accounts[GENESIS_INDEX..], - )?; - let (verified_script, verified_modules) = Self::deserialize_verified_program( - &keyed_accounts[PROGRAM_INDEX].account.data, - )?; + let data_accounts = keyed_accounts_iter.into_slice(); + + let mut data_store = Self::keyed_accounts_to_data_store(&data_accounts)?; + let (verified_script, verified_modules) = + Self::deserialize_verified_program(&program.account.data)?; let output = Self::execute( sender_address, @@ -418,7 +429,7 @@ impl MoveProcessor { } data_store.apply_write_set(&output.write_set()); - Self::data_store_to_keyed_accounts(data_store, keyed_accounts) + Self::data_store_to_keyed_accounts(data_store, data_accounts) } } } @@ -585,22 +596,27 @@ mod tests { fn test_invoke_mint_to_address() { solana_logger::setup(); - let amount = 42; - let accounts = mint_coins(amount).unwrap(); - let mut data_store = DataStore::default(); - match bincode::deserialize(&accounts[GENESIS_INDEX + 1].account.data).unwrap() { + + let amount = 42; + let mut accounts = mint_coins(amount).unwrap(); + let mut accounts_iter = accounts.iter_mut(); + + let _program = next_libra_account(&mut accounts_iter).unwrap(); + let genesis = next_libra_account(&mut accounts_iter).unwrap(); + let payee = next_libra_account(&mut accounts_iter).unwrap(); + match bincode::deserialize(&payee.account.data).unwrap() { LibraAccountState::User(owner, write_set) => { - if owner != accounts[GENESIS_INDEX].key { + if owner != genesis.key { panic!(); } data_store.apply_write_set(&write_set) } _ => panic!("Invalid account state"), } - let payee_resource = data_store - .read_account_resource(&accounts[GENESIS_INDEX + 1].address) - .unwrap(); + + let payee_resource = data_store.read_account_resource(&payee.address).unwrap(); + println!("{}:{}", line!(), file!()); assert_eq!(amount, payee_resource.balance()); assert_eq!(0, payee_resource.sequence_number()); @@ -611,6 +627,11 @@ mod tests { solana_logger::setup(); let amount_to_mint = 42; let mut accounts = mint_coins(amount_to_mint).unwrap(); + let mut accounts_iter = accounts.iter_mut(); + + let _program = next_libra_account(&mut accounts_iter).unwrap(); + let genesis = next_libra_account(&mut accounts_iter).unwrap(); + let sender = next_libra_account(&mut accounts_iter).unwrap(); let code = " import 0x0.LibraAccount; @@ -620,14 +641,9 @@ mod tests { return; } "; - let mut program = - LibraAccount::create_program(&accounts[GENESIS_INDEX + 1].address, code, vec![]); + let mut program = LibraAccount::create_program(&genesis.address, code, vec![]); let mut payee = LibraAccount::create_unallocated(); - let (genesis, sender) = accounts.split_at_mut(GENESIS_INDEX + 1); - let genesis = &mut genesis[1]; - let sender = &mut sender[0]; - let rent_id = rent::id(); let mut rent_account = rent::create_account(1, &Rent::default()); let mut keyed_accounts = vec![ @@ -662,9 +678,7 @@ mod tests { ) .unwrap(); - let data_store = - MoveProcessor::keyed_accounts_to_data_store(&genesis.key, &keyed_accounts[1..]) - .unwrap(); + let data_store = MoveProcessor::keyed_accounts_to_data_store(&keyed_accounts[1..]).unwrap(); let sender_resource = data_store.read_account_resource(&sender.address).unwrap(); let payee_resource = data_store.read_account_resource(&payee.address).unwrap(); @@ -876,6 +890,11 @@ mod tests { pub address: AccountAddress, pub account: Account, } + + pub fn next_libra_account(iter: &mut I) -> Result { + iter.next().ok_or(InstructionError::NotEnoughAccountKeys) + } + impl LibraAccount { pub fn new(key: Pubkey, account: Account) -> Self { let address = pubkey_to_address(&key); diff --git a/sdk/src/instruction.rs b/sdk/src/instruction.rs index fcf7e53051..c18ec5f8f0 100644 --- a/sdk/src/instruction.rs +++ b/sdk/src/instruction.rs @@ -73,6 +73,9 @@ pub enum InstructionError { /// A non-system program changed the size of the account data AccountDataSizeChanged, + /// the instruction expected an executable account + AccountNotExecutable, + /// CustomError allows on-chain programs to implement program-specific error types and see /// them returned by the Solana runtime. A CustomError may be any type that is represented /// as or serialized to a u32 integer.