Splits index of InstructionAccount into index_in_transaction and index_in_caller. (#22165)

This commit is contained in:
Alexander Meißner
2021-12-30 15:46:36 +01:00
committed by GitHub
parent 3f88994e0f
commit edb20d6909
7 changed files with 341 additions and 188 deletions

View File

@ -257,16 +257,19 @@ impl<'a> InvokeContext<'a> {
}
self.pre_accounts = Vec::with_capacity(instruction_accounts.len());
let mut work = |_index_in_instruction: usize, entry: &InstructionAccount| {
if entry.index < self.transaction_context.get_number_of_accounts() {
let mut work = |_index_in_instruction: usize,
instruction_account: &InstructionAccount| {
if instruction_account.index_in_transaction
< self.transaction_context.get_number_of_accounts()
{
let account = self
.transaction_context
.get_account_at_index(entry.index)
.get_account_at_index(instruction_account.index_in_transaction)
.borrow()
.clone();
self.pre_accounts.push(PreAccount::new(
self.transaction_context
.get_key_of_account_at_index(entry.index),
.get_key_of_account_at_index(instruction_account.index_in_transaction),
account,
));
return Ok(());
@ -308,9 +311,9 @@ impl<'a> InvokeContext<'a> {
instruction_account.is_signer,
instruction_account.is_writable,
self.transaction_context
.get_key_of_account_at_index(instruction_account.index),
.get_key_of_account_at_index(instruction_account.index_in_transaction),
self.transaction_context
.get_account_at_index(instruction_account.index),
.get_account_at_index(instruction_account.index_in_transaction),
)
}))
.collect::<Vec<_>>();
@ -361,7 +364,7 @@ impl<'a> InvokeContext<'a> {
// Verify account has no outstanding references
let _ = self
.transaction_context
.get_account_at_index(instruction_account.index)
.get_account_at_index(instruction_account.index_in_transaction)
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
}
@ -369,7 +372,7 @@ impl<'a> InvokeContext<'a> {
pre_account_index = pre_account_index.saturating_add(1);
let account = self
.transaction_context
.get_account_at_index(instruction_account.index)
.get_account_at_index(instruction_account.index_in_transaction)
.borrow();
pre_account
.verify(
@ -428,10 +431,13 @@ impl<'a> InvokeContext<'a> {
// Verify the per-account instruction results
let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
let mut work = |index_in_instruction: usize, instruction_account: &InstructionAccount| {
if instruction_account.index < transaction_context.get_number_of_accounts() {
let key =
transaction_context.get_key_of_account_at_index(instruction_account.index);
let account = transaction_context.get_account_at_index(instruction_account.index);
if instruction_account.index_in_transaction
< transaction_context.get_number_of_accounts()
{
let key = transaction_context
.get_key_of_account_at_index(instruction_account.index_in_transaction);
let account = transaction_context
.get_account_at_index(instruction_account.index_in_transaction);
let is_writable = if let Some(caller_write_privileges) = caller_write_privileges {
caller_write_privileges[index_in_instruction]
} else {
@ -502,11 +508,11 @@ impl<'a> InvokeContext<'a> {
for instruction_account in instruction_accounts.iter() {
let account_length = self
.transaction_context
.get_account_at_index(instruction_account.index)
.get_account_at_index(instruction_account.index_in_transaction)
.borrow()
.data()
.len();
prev_account_sizes.push((instruction_account.index, account_length));
prev_account_sizes.push((instruction_account.index_in_transaction, account_length));
}
self.process_instruction(
@ -552,10 +558,11 @@ impl<'a> InvokeContext<'a> {
// Finds the index of each account in the instruction by its pubkey.
// Then normalizes / unifies the privileges of duplicate accounts.
// Note: This works like visit_each_account_once() and is an O(n^2) algorithm too.
let caller_keyed_accounts = self.get_keyed_accounts()?;
let mut deduplicated_instruction_accounts: Vec<InstructionAccount> = Vec::new();
let mut duplicate_indicies = Vec::with_capacity(instruction.accounts.len());
for account_meta in instruction.accounts.iter() {
let account_index = self
let index_in_transaction = self
.transaction_context
.find_index_of_account(&account_meta.pubkey)
.ok_or_else(|| {
@ -566,37 +573,21 @@ impl<'a> InvokeContext<'a> {
);
InstructionError::MissingAccount
})?;
if let Some(duplicate_index) = deduplicated_instruction_accounts
.iter()
.position(|instruction_account| instruction_account.index == account_index)
if let Some(duplicate_index) =
deduplicated_instruction_accounts
.iter()
.position(|instruction_account| {
instruction_account.index_in_transaction == index_in_transaction
})
{
duplicate_indicies.push(duplicate_index);
let instruction_account = &mut deduplicated_instruction_accounts[duplicate_index];
instruction_account.is_signer |= account_meta.is_signer;
instruction_account.is_writable |= account_meta.is_writable;
} else {
duplicate_indicies.push(deduplicated_instruction_accounts.len());
deduplicated_instruction_accounts.push(InstructionAccount {
index: account_index,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
});
}
}
let instruction_accounts = duplicate_indicies
.into_iter()
.map(|duplicate_index| deduplicated_instruction_accounts[duplicate_index].clone())
.collect();
// Check for privilege escalation
let caller_keyed_accounts = self.get_instruction_keyed_accounts()?;
let caller_write_privileges = instruction
.accounts
.iter()
.map(|account_meta| {
let keyed_account = caller_keyed_accounts
let index_in_caller = caller_keyed_accounts
.iter()
.find(|keyed_account| *keyed_account.unsigned_key() == account_meta.pubkey)
.position(|keyed_account| *keyed_account.unsigned_key() == account_meta.pubkey)
.ok_or_else(|| {
ic_msg!(
self,
@ -605,24 +596,47 @@ impl<'a> InvokeContext<'a> {
);
InstructionError::MissingAccount
})?;
duplicate_indicies.push(deduplicated_instruction_accounts.len());
deduplicated_instruction_accounts.push(InstructionAccount {
index_in_transaction,
index_in_caller,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
});
}
}
let instruction_accounts: Vec<InstructionAccount> = duplicate_indicies
.into_iter()
.map(|duplicate_index| deduplicated_instruction_accounts[duplicate_index].clone())
.collect();
// Check for privilege escalation
let caller_write_privileges = instruction_accounts
.iter()
.map(|instruction_account| {
let keyed_account = &caller_keyed_accounts[instruction_account.index_in_caller];
// Readonly in caller cannot become writable in callee
if account_meta.is_writable && !keyed_account.is_writable() {
if instruction_account.is_writable && !keyed_account.is_writable() {
ic_msg!(
self,
"{}'s writable privilege escalated",
account_meta.pubkey,
keyed_account.unsigned_key(),
);
return Err(InstructionError::PrivilegeEscalation);
}
// To be signed in the callee,
// it must be either signed in the caller or by the program
if account_meta.is_signer
if instruction_account.is_signer
&& !(keyed_account.signer_key().is_some()
|| signers.contains(&account_meta.pubkey))
|| signers.contains(keyed_account.unsigned_key()))
{
ic_msg!(self, "{}'s signer privilege escalated", account_meta.pubkey);
ic_msg!(
self,
"{}'s signer privilege escalated",
keyed_account.unsigned_key()
);
return Err(InstructionError::PrivilegeEscalation);
}
@ -720,7 +734,7 @@ impl<'a> InvokeContext<'a> {
data: instruction_data.to_vec(),
accounts: instruction_accounts
.iter()
.map(|instruction_account| instruction_account.index as u8)
.map(|instruction_account| instruction_account.index_in_transaction as u8)
.collect(),
};
instruction_recorder
@ -907,16 +921,21 @@ pub struct MockInvokeContextPreparation {
pub fn prepare_mock_invoke_context(
transaction_accounts: Vec<TransactionAccount>,
instruction_accounts: Vec<AccountMeta>,
program_indices: &[usize],
) -> MockInvokeContextPreparation {
let instruction_accounts = instruction_accounts
.iter()
.map(|account_meta| InstructionAccount {
index: transaction_accounts
.map(|account_meta| {
let index_in_transaction = transaction_accounts
.iter()
.position(|(key, _account)| *key == account_meta.pubkey)
.unwrap_or(transaction_accounts.len()),
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
.unwrap_or(transaction_accounts.len());
InstructionAccount {
index_in_transaction,
index_in_caller: program_indices.len().saturating_add(index_in_transaction),
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
}
})
.collect();
MockInvokeContextPreparation {
@ -950,7 +969,8 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
is_signer: false,
is_writable: false,
}];
let preparation = prepare_mock_invoke_context(transaction_accounts, instruction_accounts);
let preparation =
prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices);
let transaction_context = TransactionContext::new(
preparation.transaction_accounts,
ComputeBudget::default().max_invoke_depth,
@ -972,9 +992,10 @@ pub fn mock_process_instruction_with_sysvars(
sysvars: &[(Pubkey, Vec<u8>)],
process_instruction: ProcessInstructionWithContext,
) -> Vec<AccountSharedData> {
let mut preparation = prepare_mock_invoke_context(transaction_accounts, instruction_accounts);
program_indices.insert(0, transaction_accounts.len());
let mut preparation =
prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices);
let processor_account = AccountSharedData::new(0, 0, &solana_sdk::native_loader::id());
program_indices.insert(0, preparation.transaction_accounts.len());
preparation
.transaction_accounts
.push((*loader_id, processor_account));
@ -1023,7 +1044,7 @@ fn visit_each_account_once(
// Note: This is an O(n^2) algorithm,
// but performed on a very small slice and requires no heap allocations
for before in instruction_accounts[..index].iter() {
if before.index == instruction_account.index {
if before.index_in_transaction == instruction_account.index_in_transaction {
continue 'root; // skip dups
}
}
@ -1058,7 +1079,7 @@ mod tests {
let mut work = |index_in_instruction: usize, entry: &InstructionAccount| {
unique_entries += 1;
index_sum_a += index_in_instruction;
index_sum_b += entry.index;
index_sum_b += entry.index_in_transaction;
Ok(())
};
visit_each_account_once(accounts, &mut work).unwrap();
@ -1070,22 +1091,26 @@ mod tests {
(3, 3, 19),
do_work(&[
InstructionAccount {
index: 7,
index_in_transaction: 7,
index_in_caller: 0,
is_signer: false,
is_writable: false,
},
InstructionAccount {
index: 3,
index_in_transaction: 3,
index_in_caller: 1,
is_signer: false,
is_writable: false,
},
InstructionAccount {
index: 9,
index_in_transaction: 9,
index_in_caller: 2,
is_signer: false,
is_writable: false,
},
InstructionAccount {
index: 3,
index_in_transaction: 3,
index_in_caller: 1,
is_signer: false,
is_writable: false,
},
@ -1180,7 +1205,8 @@ mod tests {
AccountSharedData::new(index as u64, 1, &invoke_stack[index]),
));
instruction_accounts.push(InstructionAccount {
index,
index_in_transaction: index,
index_in_caller: 1 + index,
is_signer: false,
is_writable: true,
});
@ -1191,12 +1217,13 @@ mod tests {
AccountSharedData::new(1, 1, &solana_sdk::pubkey::Pubkey::default()),
));
instruction_accounts.push(InstructionAccount {
index,
index_in_transaction: index,
index_in_caller: 1 + index,
is_signer: false,
is_writable: false,
});
}
let transaction_context = TransactionContext::new(accounts, 1);
let transaction_context = TransactionContext::new(accounts, MAX_DEPTH);
let mut invoke_context = InvokeContext::new_mock(&transaction_context, &[]);
// Check call depth increases and has a limit
@ -1217,12 +1244,14 @@ mod tests {
let not_owned_index = owned_index - 1;
let instruction_accounts = vec![
InstructionAccount {
index: not_owned_index,
index_in_transaction: not_owned_index,
index_in_caller: 1 + not_owned_index,
is_signer: false,
is_writable: true,
},
InstructionAccount {
index: owned_index,
index_in_transaction: owned_index,
index_in_caller: 1 + owned_index,
is_signer: false,
is_writable: true,
},
@ -1318,8 +1347,9 @@ mod tests {
let instruction_accounts = metas
.iter()
.enumerate()
.map(|(account_index, account_meta)| InstructionAccount {
index: account_index,
.map(|(index_in_transaction, account_meta)| InstructionAccount {
index_in_transaction,
index_in_caller: program_indices.len() + index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})
@ -1436,8 +1466,9 @@ mod tests {
let instruction_accounts = metas
.iter()
.enumerate()
.map(|(account_index, account_meta)| InstructionAccount {
index: account_index,
.map(|(index_in_transaction, account_meta)| InstructionAccount {
index_in_transaction,
index_in_caller: program_indices.len() + index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})