Don't use caller passed executable account (#14276)

This commit is contained in:
Jack May 2020-12-23 14:34:14 -08:00 committed by GitHub
parent 0b479ab180
commit b1d702a618
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 78 additions and 101 deletions

View File

@ -291,7 +291,8 @@ extern uint64_t entrypoint(const uint8_t *input) {
case TEST_EMPTY_ACCOUNTS_SLICE: {
sol_log("Empty accounts slice");
SolAccountMeta arguments[] = {};
SolAccountMeta arguments[] = {
{accounts[INVOKED_ARGUMENT_INDEX].key, false, false}};
uint8_t data[] = {};
const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
arguments, SOL_ARRAY_SIZE(arguments),

View File

@ -385,7 +385,11 @@ fn process_instruction(
}
TEST_EMPTY_ACCOUNTS_SLICE => {
msg!("Empty accounts slice");
let instruction = create_instruction(*accounts[INVOKED_PROGRAM_INDEX].key, &[], vec![]);
let instruction = create_instruction(
*accounts[INVOKED_PROGRAM_INDEX].key,
&[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)],
vec![],
);
invoke(&instruction, &[])?;
}
TEST_CAP_SEEDS => {

View File

@ -989,7 +989,7 @@ fn test_program_bpf_invoke() {
let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
assert_eq!(
result.unwrap_err().unwrap(),
TransactionError::InstructionError(0, InstructionError::ModifiedProgramId)
TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature)
);
assert_eq!(10, bank.get_balance(&from_pubkey));
assert_eq!(0, bank.get_balance(&to_pubkey));

View File

@ -23,7 +23,6 @@ use solana_sdk::{
hash::{Hasher, HASH_BYTES},
instruction::{AccountMeta, Instruction, InstructionError},
keyed_account::KeyedAccount,
message::Message,
native_loader,
process_instruction::{stable_log, ComputeMeter, InvokeContext, Logger},
program_error::ProgramError,
@ -834,7 +833,10 @@ struct AccountReferences<'a> {
ref_to_len_in_vm: &'a mut u64,
serialized_len_ptr: &'a mut u64,
}
type TranslatedAccounts<'a> = (Vec<Rc<RefCell<Account>>>, Vec<AccountReferences<'a>>);
type TranslatedAccounts<'a> = (
Vec<Rc<RefCell<Account>>>,
Vec<Option<AccountReferences<'a>>>,
);
/// Implemented by language specific data structure translators
trait SyscallInvokeSigned<'a> {
@ -847,7 +849,8 @@ trait SyscallInvokeSigned<'a> {
) -> Result<Instruction, EbpfError<BPFError>>;
fn translate_accounts(
&self,
message: &Message,
account_keys: &[Pubkey],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
memory_mapping: &MemoryMapping,
@ -905,7 +908,8 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> {
fn translate_accounts(
&self,
message: &Message,
account_keys: &[Pubkey],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
memory_mapping: &MemoryMapping,
@ -921,9 +925,13 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> {
&[]
};
let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut refs = Vec::with_capacity(message.account_keys.len());
'root: for account_key in message.account_keys.iter() {
let mut accounts = Vec::with_capacity(account_keys.len());
let mut refs = Vec::with_capacity(account_keys.len());
'root: for (i, account_key) in account_keys.iter().enumerate() {
if i == program_account_index {
// Don't look for caller passed executable, runtime already has it
continue 'root;
}
for account_info in account_infos.iter() {
let key = translate_type::<Pubkey>(
memory_mapping,
@ -986,14 +994,14 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> {
owner: *owner,
rent_epoch: account_info.rent_epoch,
})));
refs.push(AccountReferences {
refs.push(Some(AccountReferences {
lamports,
owner,
data,
vm_data_addr,
ref_to_len_in_vm,
serialized_len_ptr,
});
}));
continue 'root;
}
}
@ -1182,7 +1190,8 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> {
fn translate_accounts(
&self,
message: &Message,
account_keys: &[Pubkey],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
memory_mapping: &MemoryMapping,
@ -1193,9 +1202,13 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> {
account_infos_len,
self.loader_id,
)?;
let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut refs = Vec::with_capacity(message.account_keys.len());
'root: for account_key in message.account_keys.iter() {
let mut accounts = Vec::with_capacity(account_keys.len());
let mut refs = Vec::with_capacity(account_keys.len());
'root: for (i, account_key) in account_keys.iter().enumerate() {
if i == program_account_index {
// Don't look for caller passed executable, runtime already has it
continue 'root;
}
for account_info in account_infos.iter() {
let key = translate_type::<Pubkey>(
memory_mapping,
@ -1247,14 +1260,14 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> {
owner: *owner,
rent_epoch: account_info.rent_epoch,
})));
refs.push(AccountReferences {
refs.push(Some(AccountReferences {
lamports,
owner,
data,
vm_data_addr,
ref_to_len_in_vm,
serialized_len_ptr,
});
}));
continue 'root;
}
}
@ -1381,12 +1394,13 @@ fn call<'a>(
.get_callers_keyed_accounts()
.iter()
.collect::<Vec<&KeyedAccount>>();
let (message, callee_program_id) =
let (message, callee_program_id, program_id_index) =
MessageProcessor::create_message(&instruction, &keyed_account_refs, &signers)
.map_err(SyscallError::InstructionError)?;
is_authorized_program(&callee_program_id)?;
let (accounts, account_refs) = syscall.translate_accounts(
&message,
let (mut accounts, mut account_refs) = syscall.translate_accounts(
&message.account_keys,
program_id_index,
account_infos_addr,
account_infos_len,
memory_mapping,
@ -1426,6 +1440,8 @@ fn call<'a>(
} else {
None
};
accounts.insert(program_id_index, Rc::new(program_account.clone()));
account_refs.insert(program_id_index, None);
let mut executable_accounts = vec![(callee_program_id, program_account)];
if let Some(programdata) = programdata_executable {
executable_accounts.push(programdata);
@ -1446,9 +1462,9 @@ fn call<'a>(
}
// Copy results back to caller
for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() {
let account = account.borrow();
if let Some(account_ref) = account_ref {
if message.is_writable(i) && !account.executable {
*account_ref.lamports = account.lamports;
*account_ref.owner = account.owner;
@ -1456,14 +1472,16 @@ fn call<'a>(
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
return Err(
SyscallError::InstructionError(InstructionError::InvalidRealloc).into(),
);
return Err(SyscallError::InstructionError(
InstructionError::InvalidRealloc,
)
.into());
}
if account.data.len() > account_ref.data.len() + MAX_PERMITTED_DATA_INCREASE {
return Err(
SyscallError::InstructionError(InstructionError::InvalidRealloc).into(),
);
return Err(SyscallError::InstructionError(
InstructionError::InvalidRealloc,
)
.into());
}
let _ = translate(
memory_mapping,
@ -1479,6 +1497,7 @@ fn call<'a>(
.clone_from_slice(&account.data[0..account_ref.data.len()]);
}
}
}
Ok(SUCCESS)
}

View File

@ -509,58 +509,11 @@ impl MessageProcessor {
Err(InstructionError::UnsupportedProgramId)
}
/// Verify instruction against the caller
pub fn verify_instruction(
keyed_accounts: &[&KeyedAccount],
instruction: &Instruction,
signers: &[Pubkey],
) -> Result<(), InstructionError> {
// Check for privilege escalation
for account in instruction.accounts.iter() {
let keyed_account = keyed_accounts
.iter()
.find_map(|keyed_account| {
if &account.pubkey == keyed_account.unsigned_key() {
Some(keyed_account)
} else {
None
}
})
.ok_or(InstructionError::MissingAccount)?;
// Readonly account cannot become writable
if account.is_writable && !keyed_account.is_writable() {
return Err(InstructionError::PrivilegeEscalation);
}
if account.is_signer && // If message indicates account is signed
!( // one of the following needs to be true:
keyed_account.signer_key().is_some() // Signed in the parent instruction
|| signers.contains(&account.pubkey) // Signed by the program
) {
return Err(InstructionError::PrivilegeEscalation);
}
}
// validate the caller has access to the program account
let _ = keyed_accounts
.iter()
.find_map(|keyed_account| {
if &instruction.program_id == keyed_account.unsigned_key() {
Some(keyed_account)
} else {
None
}
})
.ok_or(InstructionError::MissingAccount)?;
Ok(())
}
pub fn create_message(
instruction: &Instruction,
keyed_accounts: &[&KeyedAccount],
signers: &[Pubkey],
) -> Result<(Message, Pubkey), InstructionError> {
) -> Result<(Message, Pubkey, usize), InstructionError> {
// Check for privilege escalation
for account in instruction.accounts.iter() {
let keyed_account = keyed_accounts
@ -588,6 +541,7 @@ impl MessageProcessor {
}
// validate the caller has access to the program account
let program_id = instruction.program_id;
let _ = keyed_accounts
.iter()
.find_map(|keyed_account| {
@ -600,10 +554,9 @@ impl MessageProcessor {
.ok_or(InstructionError::MissingAccount)?;
let message = Message::new(&[instruction.clone()], None);
let id = *message
.program_id(0)
.ok_or(InstructionError::MissingAccount)?;
Ok((message, id))
let program_id_index = message.instructions[0].program_id_index as usize;
Ok((message, program_id, program_id_index))
}
/// Entrypoint for a cross-program invocation from a native program
@ -621,7 +574,7 @@ impl MessageProcessor {
.iter()
.map(|seeds| Pubkey::create_program_address(&seeds, caller_program_id))
.collect::<Result<Vec<_>, solana_sdk::pubkey::PubkeyError>>()?;
let (message, callee_program_id) =
let (message, callee_program_id, _) =
Self::create_message(&instruction, &keyed_accounts, &signers)?;
let mut accounts = vec![];
let mut account_refs = vec![];