Skip to content

Commit

Permalink
Splits index of InstructionAccount into index_in_transaction and inde…
Browse files Browse the repository at this point in the history 
…x_in_caller.
  • Loading branch information
@Lichtso
Lichtso committed Dec 29, 2021
1 parent 57986f9 commit e62d249
Show file tree
Hide file tree
Showing 5 changed files with 288 additions and 184 deletions.
155 changes: 92 additions & 63 deletions program-runtime/src/invoke_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,16 +263,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(());
Expand Down Expand Up @@ -314,9 +317,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<_>>();
Expand Down Expand Up @@ -367,15 +370,15 @@ 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)?;
}
let pre_account = &self.pre_accounts[pre_account_index];
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(
Expand Down Expand Up @@ -434,10 +437,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 {
Expand Down Expand Up @@ -508,11 +514,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(
Expand Down Expand Up @@ -559,10 +565,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_instruction_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(|| {
Expand All @@ -573,63 +580,70 @@ 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 {
let index_in_caller = caller_keyed_accounts
.iter()
.position(|keyed_account| *keyed_account.unsigned_key() == account_meta.pubkey)
.ok_or_else(|| {
ic_msg!(
self,
"Instruction references an unknown account {}",
account_meta.pubkey,
);
InstructionError::MissingAccount
})?;
duplicate_indicies.push(deduplicated_instruction_accounts.len());
deduplicated_instruction_accounts.push(InstructionAccount {
index: account_index,
index_in_transaction,
index_in_caller,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
});
}
}
let instruction_accounts = duplicate_indicies
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_keyed_accounts = self.get_instruction_keyed_accounts()?;
let caller_write_privileges = instruction
.accounts
let caller_write_privileges = instruction_accounts
.iter()
.map(|account_meta| {
let keyed_account = caller_keyed_accounts
.iter()
.find(|keyed_account| *keyed_account.unsigned_key() == account_meta.pubkey)
.ok_or_else(|| {
ic_msg!(
self,
"Instruction references an unknown account {}",
account_meta.pubkey,
);
InstructionError::MissingAccount
})?;
.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);
}

Expand Down Expand Up @@ -732,7 +746,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
Expand Down Expand Up @@ -926,13 +940,17 @@ pub fn prepare_mock_invoke_context(
) -> 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: index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
}
})
.collect();
MockInvokeContextPreparation {
Expand Down Expand Up @@ -1039,7 +1057,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
}
}
Expand Down Expand Up @@ -1078,7 +1096,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();
Expand All @@ -1090,22 +1108,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,
},
Expand Down Expand Up @@ -1211,7 +1233,8 @@ mod tests {
AccountSharedData::new(index as u64, 1, &invoke_stack[index]),
));
instruction_accounts.push(InstructionAccount {
index,
index_in_transaction: index,
index_in_caller: index,
is_signer: false,
is_writable: true,
});
Expand All @@ -1222,12 +1245,13 @@ mod tests {
AccountSharedData::new(1, 1, &solana_sdk::pubkey::Pubkey::default()),
));
instruction_accounts.push(InstructionAccount {
index,
index_in_transaction: index,
index_in_caller: 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
Expand All @@ -1248,12 +1272,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: not_owned_index,
is_signer: false,
is_writable: true,
},
InstructionAccount {
index: owned_index,
index_in_transaction: owned_index,
index_in_caller: owned_index,
is_signer: false,
is_writable: true,
},
Expand Down Expand Up @@ -1349,8 +1375,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: index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})
Expand Down Expand Up @@ -1489,8 +1516,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: index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})
Expand Down Expand Up @@ -1642,8 +1670,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: index_in_transaction,
is_signer: account_meta.is_signer,
is_writable: account_meta.is_writable,
})
Expand Down
Loading

0 comments on commit e62d249

Please sign in to comment.