diff --git a/runtime/src/bank/builtins/core_bpf_migration/error.rs b/runtime/src/bank/builtins/core_bpf_migration/error.rs index f3059ed4509fe7..1bf64d003518c0 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/error.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/error.rs @@ -21,6 +21,9 @@ pub enum CoreBpfMigrationError { /// Incorrect account owner #[error("Incorrect account owner for {0:?}")] IncorrectOwner(Pubkey), + /// Program account not executable + #[error("Program account not executable for program {0:?}")] + ProgramAccountNotExecutable(Pubkey), /// Program has a data account #[error("Data account exists for program {0:?}")] ProgramHasDataAccount(Pubkey), diff --git a/runtime/src/bank/builtins/core_bpf_migration/mod.rs b/runtime/src/bank/builtins/core_bpf_migration/mod.rs index 871b173e5fa772..6fecbdeba3640e 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/mod.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/mod.rs @@ -88,7 +88,9 @@ impl Bank { }; let lamports = self.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); - let account = AccountSharedData::new_data(lamports, &state, &bpf_loader_upgradeable::id())?; + let mut account = + AccountSharedData::new_data(lamports, &state, &bpf_loader_upgradeable::id())?; + account.set_executable(true); Ok(account) } @@ -557,6 +559,9 @@ pub(crate) mod tests { // Program account is owned by the upgradeable loader. assert_eq!(program_account.owner(), &bpf_loader_upgradeable::id()); + // Program account is executable. + assert!(program_account.executable()); + // Program account has the correct state, with a pointer to its program // data address. let program_account_state: UpgradeableLoaderState = program_account.state().unwrap(); @@ -887,6 +892,7 @@ pub(crate) mod tests { let owner = &bpf_loader_upgradeable::id(); let mut account = AccountSharedData::new(lamports, space, owner); + account.set_executable(true); account.data_as_mut_slice().copy_from_slice(&data); bank.store_account_and_update_capitalization(program_address, &account); account diff --git a/runtime/src/bank/builtins/core_bpf_migration/target_core_bpf.rs b/runtime/src/bank/builtins/core_bpf_migration/target_core_bpf.rs index a98a72528bb567..2e1e3ff484a82a 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/target_core_bpf.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/target_core_bpf.rs @@ -20,7 +20,8 @@ pub(crate) struct TargetCoreBpf { impl TargetCoreBpf { /// Collects the details of a Core BPF program and verifies it is properly /// configured. - /// The program account should exist with a pointer to its data account. + /// The program account should exist with a pointer to its data account + /// and it should be marked as executable. /// The program data account should exist with the correct state /// (a ProgramData header and the program ELF). pub(crate) fn new_checked( @@ -39,6 +40,13 @@ impl TargetCoreBpf { return Err(CoreBpfMigrationError::IncorrectOwner(*program_address)); } + // The program account should be executable. + if !program_account.executable() { + return Err(CoreBpfMigrationError::ProgramAccountNotExecutable( + *program_address, + )); + } + // The program account should have a pointer to its data account. match program_account.deserialize_data::()? { UpgradeableLoaderState::Program { @@ -94,11 +102,11 @@ mod tests { solana_sdk::{account::WritableAccount, bpf_loader_upgradeable}, }; - fn store_account(bank: &Bank, address: &Pubkey, data: &[u8], owner: &Pubkey) { + fn store_account(bank: &Bank, address: &Pubkey, data: &[u8], owner: &Pubkey, executable: bool) { let space = data.len(); let lamports = bank.get_minimum_balance_for_rent_exemption(space); let mut account = AccountSharedData::new(lamports, space, owner); - account.set_executable(true); + account.set_executable(executable); account.data_as_mut_slice().copy_from_slice(data); bank.store_account_and_update_capitalization(address, &account); } @@ -125,18 +133,36 @@ mod tests { }) .unwrap(), &Pubkey::new_unique(), // Not the upgradeable loader + true, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), CoreBpfMigrationError::IncorrectOwner(..) ); + // Fail if the program account is not executable. + store_account( + &bank, + &program_address, + &bincode::serialize(&UpgradeableLoaderState::Program { + programdata_address: program_data_address, + }) + .unwrap(), + &bpf_loader_upgradeable::id(), + false, // Not executable + ); + assert_matches!( + TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), + CoreBpfMigrationError::ProgramAccountNotExecutable(..) + ); + // Fail if the program account does not have the correct state. store_account( &bank, &program_address, &[4u8; 200], // Not the correct state &bpf_loader_upgradeable::id(), + true, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), @@ -154,6 +180,7 @@ mod tests { }) .unwrap(), &bpf_loader_upgradeable::id(), + true, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), @@ -171,6 +198,7 @@ mod tests { }) .unwrap(), &bpf_loader_upgradeable::id(), + true, ); // Store the proper program account. @@ -182,6 +210,7 @@ mod tests { }) .unwrap(), &bpf_loader_upgradeable::id(), + true, ); // Fail if the program data account does not exist. @@ -200,6 +229,7 @@ mod tests { }) .unwrap(), &Pubkey::new_unique(), // Not the upgradeable loader + false, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), @@ -212,6 +242,7 @@ mod tests { &program_data_address, &[4u8; 200], // Not the correct state &bpf_loader_upgradeable::id(), + false, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), @@ -228,6 +259,7 @@ mod tests { }) .unwrap(), &bpf_loader_upgradeable::id(), + false, ); assert_matches!( TargetCoreBpf::new_checked(&bank, &program_address).unwrap_err(), @@ -257,6 +289,7 @@ mod tests { &program_data_address, &data, &bpf_loader_upgradeable::id(), + false, ); let target_core_bpf = TargetCoreBpf::new_checked(&bank, &program_address).unwrap();