gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Prevent bpf loader impersonators (bp #14278) (#14353)

Browse Source 
* Prevent bpf loader impersonators (#14278)

(cherry picked from commit ee0a80a092)

# Conflicts:
#	programs/bpf_loader/src/lib.rs
#	runtime/src/message_processor.rs

* resolve conflicts

Co-authored-by: Jack May <jack@solana.com>
This commit is contained in:
mergify[bot]
2020-12-30 20:10:13 +00:00
committed by GitHub
parent a9ee6ef2d1
commit cfb28a1b2e
3 changed files with 132 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
programs/bpf/tests/programs.rs
View File

@@ -1570,3 +1570,45 @@ fn test_program_bpf_invoke_upgradeable_via_cpi() {
TransactionError::InstructionError(0, InstructionError::Custom(42)) TransactionError::InstructionError(0, InstructionError::Custom(42))
); );
} }
#[test]
#[cfg(any(feature = "bpf_c", feature = "bpf_rust"))]
fn test_program_bpf_disguised_as_bpf_loader() {
solana_logger::setup();
let mut programs = Vec::new();
#[cfg(feature = "bpf_c")]
{
programs.extend_from_slice(&[("noop")]);
}
#[cfg(feature = "bpf_rust")]
{
programs.extend_from_slice(&[("noop")]);
}
for program in programs.iter() {
let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(50);
let mut bank = Bank::new(&genesis_config);
let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!();
bank.add_builtin(&name, id, entrypoint);
let bank_client = BankClient::new(bank);
let program_id = load_bpf_program(
&bank_client,
&bpf_loader_deprecated::id(),
&mint_keypair,
program,
);
let account_metas = vec![AccountMeta::new_readonly(program_id, false)];
let instruction = Instruction::new(bpf_loader_deprecated::id(), &1u8, account_metas);
let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
assert_eq!(
result.unwrap_err().unwrap(),
TransactionError::InstructionError(0, InstructionError::IncorrectProgramId)
);
}
}

74
programs/bpf_loader/src/lib.rs
View File

@@ -126,6 +126,12 @@ fn write_program_data(
Ok(()) Ok(())
} }
fn check_loader_id(id: &Pubkey) -> bool {
bpf_loader::check_id(id)
|| bpf_loader_deprecated::check_id(id)
|| bpf_loader_upgradeable::check_id(id)
}
/// Create the BPF virtual machine /// Create the BPF virtual machine
pub fn create_vm<'a>( pub fn create_vm<'a>(
loader_id: &'a Pubkey, loader_id: &'a Pubkey,
@@ -154,18 +160,16 @@ pub fn process_instruction(
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
let logger = invoke_context.get_logger(); let logger = invoke_context.get_logger();
if !(bpf_loader::check_id(program_id)
|| bpf_loader_deprecated::check_id(program_id)
|| bpf_loader_upgradeable::check_id(program_id))
{
log!(logger, "Invalid BPF loader id");
return Err(InstructionError::IncorrectProgramId);
}
let account_iter = &mut keyed_accounts.iter(); let account_iter = &mut keyed_accounts.iter();
let first_account = next_keyed_account(account_iter)?; let first_account = next_keyed_account(account_iter)?;
if first_account.executable()? { if first_account.executable()? {
let (program, keyed_accounts, offset) = if bpf_loader_upgradeable::check_id(program_id) { if first_account.unsigned_key() != program_id {
log!(logger, "Program id mismatch");
return Err(InstructionError::IncorrectProgramId);
}
let (program, keyed_accounts, offset) =
if bpf_loader_upgradeable::check_id(&first_account.owner()?) {
if let UpgradeableLoaderState::Program { if let UpgradeableLoaderState::Program {
programdata_address, programdata_address,
} = first_account.state()? } = first_account.state()?
@@ -188,7 +192,9 @@ pub fn process_instruction(
(first_account, keyed_accounts, 0) (first_account, keyed_accounts, 0)
}; };
if program.owner()? != *program_id { let loader_id = &program.owner()?;
if !check_loader_id(loader_id) {
log!(logger, "Executable account not owned by the BPF loader"); log!(logger, "Executable account not owned by the BPF loader");
return Err(InstructionError::IncorrectProgramId); return Err(InstructionError::IncorrectProgramId);
} }
@@ -201,8 +207,14 @@ pub fn process_instruction(
invoke_context, invoke_context,
)?, )?,
}; };
executor.execute(program_id, keyed_accounts, instruction_data, invoke_context)? executor.execute(loader_id, keyed_accounts, instruction_data, invoke_context)?
} else if bpf_loader_upgradeable::check_id(program_id) { } else {
if !check_loader_id(program_id) {
log!(logger, "Invalid BPF loader id");
return Err(InstructionError::IncorrectProgramId);
}
if bpf_loader_upgradeable::check_id(program_id) {
process_loader_upgradeable_instruction( process_loader_upgradeable_instruction(
program_id, program_id,
keyed_accounts, keyed_accounts,
@@ -210,7 +222,13 @@ pub fn process_instruction(
invoke_context, invoke_context,
)?; )?;
} else { } else {
process_loader_instruction(program_id, keyed_accounts, instruction_data, invoke_context)?; process_loader_instruction(
program_id,
keyed_accounts,
instruction_data,
invoke_context,
)?;
}
} }
Ok(()) Ok(())
} }
@@ -568,7 +586,7 @@ impl Debug for BPFExecutor {
impl Executor for BPFExecutor { impl Executor for BPFExecutor {
fn execute( fn execute(
&self, &self,
program_id: &Pubkey, loader_id: &Pubkey,
keyed_accounts: &[KeyedAccount], keyed_accounts: &[KeyedAccount],
instruction_data: &[u8], instruction_data: &[u8],
invoke_context: &mut dyn InvokeContext, invoke_context: &mut dyn InvokeContext,
@@ -581,7 +599,7 @@ impl Executor for BPFExecutor {
let parameter_accounts = keyed_accounts_iter.as_slice(); let parameter_accounts = keyed_accounts_iter.as_slice();
let parameter_bytes = serialize_parameters( let parameter_bytes = serialize_parameters(
program_id, loader_id,
program.unsigned_key(), program.unsigned_key(),
parameter_accounts, parameter_accounts,
&instruction_data, &instruction_data,
@@ -589,7 +607,7 @@ impl Executor for BPFExecutor {
{ {
let compute_meter = invoke_context.get_compute_meter(); let compute_meter = invoke_context.get_compute_meter();
let (mut vm, heap_region) = match create_vm( let (mut vm, heap_region) = match create_vm(
program_id, loader_id,
self.executable.as_ref(), self.executable.as_ref(),
&parameter_accounts, &parameter_accounts,
invoke_context, invoke_context,
@@ -648,7 +666,7 @@ impl Executor for BPFExecutor {
} }
} }
} }
deserialize_parameters(program_id, parameter_accounts, &parameter_bytes)?; deserialize_parameters(loader_id, parameter_accounts, &parameter_bytes)?;
stable_log::program_success(&logger, program.unsigned_key()); stable_log::program_success(&logger, program.unsigned_key());
Ok(()) Ok(())
} }
@@ -871,20 +889,20 @@ mod tests {
// Case: Empty keyed accounts // Case: Empty keyed accounts
assert_eq!( assert_eq!(
Err(InstructionError::NotEnoughAccountKeys), Err(InstructionError::NotEnoughAccountKeys),
process_instruction(&bpf_loader::id(), &[], &[], &mut invoke_context) process_instruction(&program_id, &[], &[], &mut invoke_context)
); );
// Case: Only a program account // Case: Only a program account
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context)
); );
// Case: Account not program // Case: Account not a program
keyed_accounts[0].account.borrow_mut().executable = false; keyed_accounts[0].account.borrow_mut().executable = false;
assert_eq!( assert_eq!(
Err(InstructionError::InvalidInstructionData), Err(InstructionError::InvalidInstructionData),
process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) process_instruction(&program_id, &keyed_accounts, &[], &mut invoke_context)
); );
keyed_accounts[0].account.borrow_mut().executable = true; keyed_accounts[0].account.borrow_mut().executable = true;
@@ -893,7 +911,7 @@ mod tests {
keyed_accounts.push(KeyedAccount::new(&program_key, false, &parameter_account)); keyed_accounts.push(KeyedAccount::new(&program_key, false, &parameter_account));
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context)
); );
// Case: limited budget // Case: limited budget
@@ -924,7 +942,7 @@ mod tests {
); );
assert_eq!( assert_eq!(
Err(InstructionError::ProgramFailedToComplete), Err(InstructionError::ProgramFailedToComplete),
process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context)
); );
// Case: With duplicate accounts // Case: With duplicate accounts
@@ -936,7 +954,7 @@ mod tests {
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction( process_instruction(
&bpf_loader::id(), &program_key,
&keyed_accounts, &keyed_accounts,
&[], &[],
&mut MockInvokeContext::default() &mut MockInvokeContext::default()
@@ -964,7 +982,7 @@ mod tests {
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction( process_instruction(
&bpf_loader_deprecated::id(), &program_key,
&keyed_accounts, &keyed_accounts,
&[], &[],
&mut MockInvokeContext::default() &mut MockInvokeContext::default()
@@ -980,7 +998,7 @@ mod tests {
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction( process_instruction(
&bpf_loader_deprecated::id(), &program_key,
&keyed_accounts, &keyed_accounts,
&[], &[],
&mut MockInvokeContext::default() &mut MockInvokeContext::default()
@@ -1008,7 +1026,7 @@ mod tests {
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction( process_instruction(
&bpf_loader::id(), &program_key,
&keyed_accounts, &keyed_accounts,
&[], &[],
&mut MockInvokeContext::default() &mut MockInvokeContext::default()
@@ -1024,7 +1042,7 @@ mod tests {
assert_eq!( assert_eq!(
Ok(()), Ok(()),
process_instruction( process_instruction(
&bpf_loader::id(), &program_key,
&keyed_accounts, &keyed_accounts,
&[], &[],
&mut MockInvokeContext::default() &mut MockInvokeContext::default()

24
runtime/src/message_processor.rs
View File

@@ -466,18 +466,19 @@ impl MessageProcessor {
/// This method calls the instruction's program entrypoint method /// This method calls the instruction's program entrypoint method
fn process_instruction( fn process_instruction(
&self, &self,
program_id: &Pubkey,
keyed_accounts: &[KeyedAccount], keyed_accounts: &[KeyedAccount],
instruction_data: &[u8], instruction_data: &[u8],
invoke_context: &mut dyn InvokeContext, invoke_context: &mut dyn InvokeContext,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
if let Some(root_account) = keyed_accounts.iter().next() { if let Some(root_account) = keyed_accounts.iter().next() {
if native_loader::check_id(&root_account.owner()?) {
let root_id = root_account.unsigned_key(); let root_id = root_account.unsigned_key();
if native_loader::check_id(&root_account.owner()?) {
for (id, process_instruction) in &self.programs { for (id, process_instruction) in &self.programs {
if id == root_id { if id == root_id {
// Call the builtin program // Call the builtin program
return process_instruction( return process_instruction(
&root_id, &program_id,
&keyed_accounts[1..], &keyed_accounts[1..],
instruction_data, instruction_data,
invoke_context, invoke_context,
@@ -486,7 +487,7 @@ impl MessageProcessor {
} }
// Call the program via the native loader // Call the program via the native loader
return self.native_loader.process_instruction( return self.native_loader.process_instruction(
&native_loader::id(), program_id,
keyed_accounts, keyed_accounts,
instruction_data, instruction_data,
invoke_context, invoke_context,
@@ -497,7 +498,7 @@ impl MessageProcessor {
if id == owner_id { if id == owner_id {
// Call the program via a builtin loader // Call the program via a builtin loader
return process_instruction( return process_instruction(
&owner_id, &program_id,
keyed_accounts, keyed_accounts,
instruction_data, instruction_data,
invoke_context, invoke_context,
@@ -703,6 +704,8 @@ impl MessageProcessor {
invoke_context: &mut dyn InvokeContext, invoke_context: &mut dyn InvokeContext,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
if let Some(instruction) = message.instructions.get(0) { if let Some(instruction) = message.instructions.get(0) {
let program_id = instruction.program_id(&message.account_keys);
// Verify the calling program hasn't misbehaved // Verify the calling program hasn't misbehaved
invoke_context.verify_and_update(message, instruction, accounts)?; invoke_context.verify_and_update(message, instruction, accounts)?;
@@ -711,7 +714,7 @@ impl MessageProcessor {
Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?; Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?;
// Invoke callee // Invoke callee
invoke_context.push(instruction.program_id(&message.account_keys))?; invoke_context.push(program_id)?;
let mut message_processor = MessageProcessor::default(); let mut message_processor = MessageProcessor::default();
for (program_id, process_instruction) in invoke_context.get_programs().iter() { for (program_id, process_instruction) in invoke_context.get_programs().iter() {
@@ -719,6 +722,7 @@ impl MessageProcessor {
} }
let mut result = message_processor.process_instruction( let mut result = message_processor.process_instruction(
program_id,
&keyed_accounts, &keyed_accounts,
&instruction.data, &instruction.data,
invoke_context, invoke_context,
@@ -887,8 +891,9 @@ impl MessageProcessor {
} }
let pre_accounts = Self::create_pre_accounts(message, instruction, accounts); let pre_accounts = Self::create_pre_accounts(message, instruction, accounts);
let program_id = instruction.program_id(&message.account_keys);
let mut invoke_context = ThisInvokeContext::new( let mut invoke_context = ThisInvokeContext::new(
instruction.program_id(&message.account_keys), program_id,
rent_collector.rent, rent_collector.rent,
pre_accounts, pre_accounts,
account_deps, account_deps,
@@ -901,7 +906,12 @@ impl MessageProcessor {
); );
let keyed_accounts = let keyed_accounts =
Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?; Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?;
self.process_instruction(&keyed_accounts, &instruction.data, &mut invoke_context)?; self.process_instruction(
program_id,
&keyed_accounts,
&instruction.data,
&mut invoke_context,
)?;
Self::verify( Self::verify(
message, message,
instruction, instruction,