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

Track account writable deescalation (#14626)

Browse Source
This commit is contained in:
Jack May
2021-01-22 15:28:01 -08:00
committed by GitHub
parent cbb9ac19b9
commit 77572a7c53
14 changed files with 246 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

118
runtime/src/message_processor.rs
View File

@ -8,7 +8,7 @@ use solana_sdk::{
account::Account,
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
feature_set::{instructions_sysvar_enabled, FeatureSet},
feature_set::{instructions_sysvar_enabled, track_writable_deescalation, FeatureSet},
instruction::{CompiledInstruction, Instruction, InstructionError},
keyed_account::{create_keyed_readonly_accounts, KeyedAccount},
message::Message,
@ -51,15 +51,13 @@ impl Executors {
#[derive(Clone, Debug, Default)]
pub struct PreAccount {
key: Pubkey,
is_signer: bool,
is_writable: bool,
account: RefCell<Account>,
}
impl PreAccount {
pub fn new(key: &Pubkey, account: &Account, is_signer: bool, is_writable: bool) -> Self {
pub fn new(key: &Pubkey, account: &Account, is_writable: bool) -> Self {
Self {
key: *key,
is_signer,
is_writable,
account: RefCell::new(account.clone()),
}
@ -68,17 +66,24 @@ impl PreAccount {
pub fn verify(
&self,
program_id: &Pubkey,
is_writable: Option<bool>,
rent: &Rent,
post: &Account,
) -> Result<(), InstructionError> {
let pre = self.account.borrow();
let is_writable = if let Some(is_writable) = is_writable {
is_writable
} else {
self.is_writable
};
// Only the owner of the account may change owner and
// only if the account is writable and
// only if the account is not executable and
// only if the data is zero-initialized or empty
if pre.owner != post.owner
&& (!self.is_writable // line coverage used to get branch coverage
&& (!is_writable // line coverage used to get branch coverage
|| pre.executable
|| *program_id != pre.owner
|| !Self::is_zeroed(&post.data))
@ -95,7 +100,7 @@ impl PreAccount {
// The balance of read-only and executable accounts may not change
if pre.lamports != post.lamports {
if !self.is_writable {
if !is_writable {
return Err(InstructionError::ReadonlyLamportChange);
}
if pre.executable {
@ -116,13 +121,13 @@ impl PreAccount {
// and if the account is writable
// and if the account is not executable
if !(*program_id == pre.owner
&& self.is_writable // line coverage used to get branch coverage
&& is_writable // line coverage used to get branch coverage
&& !pre.executable)
&& pre.data != post.data
{
if pre.executable {
return Err(InstructionError::ExecutableDataModified);
} else if self.is_writable {
} else if is_writable {
return Err(InstructionError::ExternalAccountDataModified);
} else {
return Err(InstructionError::ReadonlyDataModified);
@ -134,7 +139,7 @@ impl PreAccount {
if !rent.is_exempt(post.lamports, post.data.len()) {
return Err(InstructionError::ExecutableAccountNotRentExempt);
}
if !self.is_writable // line coverage used to get branch coverage
if !is_writable // line coverage used to get branch coverage
|| pre.executable
|| *program_id != pre.owner
{
@ -268,15 +273,20 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
message: &Message,
instruction: &CompiledInstruction,
accounts: &[Rc<RefCell<Account>>],
caller_privileges: Option<&[bool]>,
) -> Result<(), InstructionError> {
let track_writable_deescalation =
self.is_feature_active(&track_writable_deescalation::id());
match self.program_ids.last() {
Some(key) => MessageProcessor::verify_and_update(
Some(program_id) => MessageProcessor::verify_and_update(
message,
instruction,
&mut self.pre_accounts,
accounts,
key,
program_id,
&self.rent,
track_writable_deescalation,
caller_privileges,
),
None => Err(InstructionError::GenericError), // Should never happen
}
@ -577,6 +587,11 @@ impl MessageProcessor {
.iter()
.map(|seeds| Pubkey::create_program_address(&seeds, caller_program_id))
.collect::<Result<Vec<_>, solana_sdk::pubkey::PubkeyError>>()?;
let mut caller_privileges = keyed_accounts
.iter()
.map(|keyed_account| keyed_account.is_writable())
.collect::<Vec<bool>>();
caller_privileges.insert(0, false);
let (message, callee_program_id, _) =
Self::create_message(&instruction, &keyed_accounts, &signers)?;
let mut accounts = vec![];
@ -628,6 +643,7 @@ impl MessageProcessor {
&message,
&executable_accounts,
&accounts,
&caller_privileges,
invoke_context,
)?;
@ -656,13 +672,19 @@ impl MessageProcessor {
message: &Message,
executable_accounts: &[(Pubkey, RefCell<Account>)],
accounts: &[Rc<RefCell<Account>>],
caller_privileges: &[bool],
invoke_context: &mut dyn InvokeContext,
) -> Result<(), InstructionError> {
if let Some(instruction) = message.instructions.get(0) {
let program_id = instruction.program_id(&message.account_keys);
// Verify the calling program hasn't misbehaved
invoke_context.verify_and_update(message, instruction, accounts)?;
invoke_context.verify_and_update(
message,
instruction,
accounts,
Some(caller_privileges),
)?;
// Construct keyed accounts
let keyed_accounts =
@ -684,7 +706,7 @@ impl MessageProcessor {
);
if result.is_ok() {
// Verify the called program has not misbehaved
result = invoke_context.verify_and_update(message, instruction, accounts);
result = invoke_context.verify_and_update(message, instruction, accounts, None);
}
invoke_context.pop();
@ -706,10 +728,9 @@ impl MessageProcessor {
{
let mut work = |_unique_index: usize, account_index: usize| {
let key = &message.account_keys[account_index];
let is_signer = account_index < message.header.num_required_signatures as usize;
let is_writable = message.is_writable(account_index);
let account = accounts[account_index].borrow();
pre_accounts.push(PreAccount::new(key, &account, is_signer, is_writable));
pre_accounts.push(PreAccount::new(key, &account, is_writable));
Ok(())
};
let _ = instruction.visit_each_account(&mut work);
@ -750,7 +771,12 @@ impl MessageProcessor {
let account = accounts[account_index]
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
pre_accounts[unique_index].verify(&program_id, rent, &account)?;
pre_accounts[unique_index].verify(
&program_id,
Some(message.is_writable(account_index)),
rent,
&account,
)?;
pre_sum += u128::from(pre_accounts[unique_index].lamports());
post_sum += u128::from(account.lamports);
Ok(())
@ -773,6 +799,8 @@ impl MessageProcessor {
accounts: &[Rc<RefCell<Account>>],
program_id: &Pubkey,
rent: &Rent,
track_writable_deescalation: bool,
caller_privileges: Option<&[bool]>,
) -> Result<(), InstructionError> {
// Verify the per-account instruction results
let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
@ -780,6 +808,15 @@ impl MessageProcessor {
if account_index < message.account_keys.len() && account_index < accounts.len() {
let key = &message.account_keys[account_index];
let account = &accounts[account_index];
let is_writable = if track_writable_deescalation {
Some(if let Some(caller_privileges) = caller_privileges {
caller_privileges[account_index]
} else {
message.is_writable(account_index)
})
} else {
None
};
// Find the matching PreAccount
for pre_account in pre_accounts.iter_mut() {
if *key == pre_account.key() {
@ -788,7 +825,7 @@ impl MessageProcessor {
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
pre_account.verify(&program_id, &rent, &account)?;
pre_account.verify(&program_id, is_writable, &rent, &account)?;
pre_sum += u128::from(pre_account.lamports());
post_sum += u128::from(account.lamports);
@ -943,16 +980,11 @@ mod tests {
1,
&program_ids[i],
))));
pre_accounts.push(PreAccount::new(
&keys[i],
&accounts[i].borrow(),
false,
true,
))
pre_accounts.push(PreAccount::new(&keys[i], &accounts[i].borrow(), false))
}
let account = Account::new(1, 1, &solana_sdk::pubkey::Pubkey::default());
for program_id in program_ids.iter() {
pre_accounts.push(PreAccount::new(program_id, &account.clone(), false, true));
pre_accounts.push(PreAccount::new(program_id, &account.clone(), false));
}
let mut invoke_context = ThisInvokeContext::new(
@ -1000,7 +1032,7 @@ mod tests {
&solana_sdk::pubkey::Pubkey::default(),
))));
invoke_context
.verify_and_update(&message, &message.instructions[0], &these_accounts)
.verify_and_update(&message, &message.instructions[0], &these_accounts, None)
.unwrap();
assert_eq!(
invoke_context.pre_accounts[owned_index]
@ -1018,6 +1050,7 @@ mod tests {
&message,
&message.instructions[0],
&accounts[not_owned_index..owned_index + 1],
None
),
Err(InstructionError::ExternalAccountDataModified)
);
@ -1074,6 +1107,7 @@ mod tests {
struct Change {
program_id: Pubkey,
is_writable: bool,
rent: Rent,
pre: PreAccount,
post: Account,
@ -1083,6 +1117,7 @@ mod tests {
Self {
program_id: *program_id,
rent: Rent::default(),
is_writable: true,
pre: PreAccount::new(
&solana_sdk::pubkey::new_rand(),
&Account {
@ -1092,7 +1127,6 @@ mod tests {
..Account::default()
},
false,
true,
),
post: Account {
owner: *owner,
@ -1102,7 +1136,7 @@ mod tests {
}
}
pub fn read_only(mut self) -> Self {
self.pre.is_writable = false;
self.is_writable = false;
self
}
pub fn executable(mut self, pre: bool, post: bool) -> Self {
@ -1130,7 +1164,12 @@ mod tests {
self
}
pub fn verify(&self) -> Result<(), InstructionError> {
self.pre.verify(&self.program_id, &self.rent, &self.post)
self.pre.verify(
&self.program_id,
Some(self.is_writable),
&self.rent,
&self.post,
)
}
}
@ -1760,7 +1799,7 @@ mod tests {
#[test]
fn test_process_cross_program() {
#[derive(Serialize, Deserialize)]
#[derive(Debug, Serialize, Deserialize)]
enum MockInstruction {
NoopSuccess,
NoopFail,
@ -1802,17 +1841,16 @@ mod tests {
let mut program_account = Account::new(1, 0, &native_loader::id());
program_account.executable = true;
let executable_preaccount =
PreAccount::new(&callee_program_id, &program_account, false, true);
let executable_preaccount = PreAccount::new(&callee_program_id, &program_account, true);
let executable_accounts = vec![(callee_program_id, RefCell::new(program_account.clone()))];
let owned_key = solana_sdk::pubkey::new_rand();
let owned_account = Account::new(42, 1, &callee_program_id);
let owned_preaccount = PreAccount::new(&owned_key, &owned_account, false, true);
let owned_preaccount = PreAccount::new(&owned_key, &owned_account, true);
let not_owned_key = solana_sdk::pubkey::new_rand();
let not_owned_account = Account::new(84, 1, &solana_sdk::pubkey::new_rand());
let not_owned_preaccount = PreAccount::new(&not_owned_key, &not_owned_account, false, true);
let not_owned_preaccount = PreAccount::new(&not_owned_key, &not_owned_account, true);
#[allow(unused_mut)]
let mut accounts = vec![
@ -1851,11 +1889,18 @@ mod tests {
metas.clone(),
);
let message = Message::new(&[instruction], None);
let caller_privileges = message
.account_keys
.iter()
.enumerate()
.map(|(i, _)| message.is_writable(i))
.collect::<Vec<bool>>();
assert_eq!(
MessageProcessor::process_cross_program_instruction(
&message,
&executable_accounts,
&accounts,
&caller_privileges,
&mut invoke_context,
),
Err(InstructionError::ExternalAccountDataModified)
@ -1878,11 +1923,18 @@ mod tests {
for case in cases {
let instruction = Instruction::new(callee_program_id, &case.0, metas.clone());
let message = Message::new(&[instruction], None);
let caller_privileges = message
.account_keys
.iter()
.enumerate()
.map(|(i, _)| message.is_writable(i))
.collect::<Vec<bool>>();
assert_eq!(
MessageProcessor::process_cross_program_instruction(
&message,
&executable_accounts,
&accounts,
&caller_privileges,
&mut invoke_context,
),
case.1