Skip to content

Commit

Permalink
Fix native invoke writable privileges
Browse files Browse the repository at this point in the history
  • Loading branch information
@jackcmay
jackcmay committed Sep 10, 2021
1 parent 0b64bf5 commit 84c5619
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 22 deletions.
31 changes: 16 additions & 15 deletions program-runtime/src/instruction_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,27 +498,28 @@ impl InstructionProcessor {
let invoke_context = invoke_context.borrow();

// Translate and verify caller's data
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let keyed_accounts = keyed_account_indices
let caller_keyed_accounts = invoke_context.get_keyed_accounts()?;
let callee_keyed_accounts = keyed_account_indices
.iter()
.map(|index| keyed_account_at_index(keyed_accounts, *index))
.map(|index| keyed_account_at_index(caller_keyed_accounts, *index))
.collect::<Result<Vec<&KeyedAccount>, InstructionError>>()?;
let (message, callee_program_id, _) =
Self::create_message(&instruction, &keyed_accounts, signers, &invoke_context)?;
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let mut caller_write_privileges = keyed_account_indices
.iter()
.map(|index| keyed_accounts[*index].is_writable())
.collect::<Vec<bool>>();
caller_write_privileges.insert(0, false);
let mut accounts = vec![];
let mut keyed_account_indices_reordered = vec![];
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let (message, callee_program_id, _) = Self::create_message(
&instruction,
&callee_keyed_accounts,
signers,
&invoke_context,
)?;

let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len());
let mut keyed_account_indices_reordered =
Vec::with_capacity(message.account_keys.len());
'root: for account_key in message.account_keys.iter() {
for keyed_account_index in keyed_account_indices {
let keyed_account = &keyed_accounts[*keyed_account_index];
let keyed_account = &caller_keyed_accounts[*keyed_account_index];
if account_key == keyed_account.unsigned_key() {
accounts.push((*account_key, Rc::new(keyed_account.account.clone())));
caller_write_privileges.push(keyed_account.is_writable());
keyed_account_indices_reordered.push(*keyed_account_index);
continue 'root;
}
Expand Down
234 changes: 227 additions & 7 deletions runtime/src/message_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,7 @@ mod tests {
NoopFail,
ModifyOwned,
ModifyNotOwned,
ModifyReadonly,
}

fn mock_process_instruction(
Expand All @@ -1151,6 +1152,9 @@ mod tests {
MockInstruction::ModifyNotOwned => {
keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
}
MockInstruction::ModifyReadonly => {
keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
}
}
} else {
return Err(InstructionError::InvalidInstructionData);
Expand All @@ -1170,6 +1174,7 @@ mod tests {

let owned_account = AccountSharedData::new(42, 1, &callee_program_id);
let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand());
let readonly_account = AccountSharedData::new(168, 1, &caller_program_id);

#[allow(unused_mut)]
let accounts = vec![
Expand All @@ -1181,23 +1186,28 @@ mod tests {
solana_sdk::pubkey::new_rand(),
Rc::new(RefCell::new(not_owned_account)),
),
(
solana_sdk::pubkey::new_rand(),
Rc::new(RefCell::new(readonly_account)),
),
(callee_program_id, Rc::new(RefCell::new(program_account))),
];

let compiled_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2]);
let programs: Vec<(_, ProcessInstructionWithContext)> =
vec![(callee_program_id, mock_process_instruction)];
let metas = vec![
AccountMeta::new(accounts[0].0, false),
AccountMeta::new(accounts[1].0, false),
AccountMeta::new_readonly(accounts[2].0, false),
];

let instruction = Instruction::new_with_bincode(
let caller_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2, 3]);
let callee_instruction = Instruction::new_with_bincode(
callee_program_id,
&MockInstruction::NoopSuccess,
metas.clone(),
);
let message = Message::new(&[instruction], None);
let message = Message::new(&[callee_instruction], None);
let feature_set = FeatureSet::all_enabled();
let demote_program_write_locks = feature_set.is_active(&demote_program_write_locks::id());

Expand All @@ -1208,7 +1218,7 @@ mod tests {
&caller_program_id,
Rent::default(),
&message,
&compiled_instruction,
&caller_instruction,
&executable_accounts,
&accounts,
programs.as_slice(),
Expand Down Expand Up @@ -1244,6 +1254,20 @@ mod tests {
);
accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0;

// readonly account modified by the invoker
accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1;
assert_eq!(
InstructionProcessor::process_cross_program_instruction(
&message,
&executable_accounts,
&accounts,
&caller_write_privileges,
&mut invoke_context,
),
Err(InstructionError::ReadonlyDataModified)
);
accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0;

let cases = vec![
(MockInstruction::NoopSuccess, Ok(())),
(
Expand All @@ -1258,9 +1282,9 @@ mod tests {
];

for case in cases {
let instruction =
let callee_instruction =
Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone());
let message = Message::new(&[instruction], None);
let message = Message::new(&[callee_instruction], None);

let ancestors = Ancestors::default();
let blockhash = Hash::default();
Expand All @@ -1269,7 +1293,7 @@ mod tests {
&caller_program_id,
Rent::default(),
&message,
&compiled_instruction,
&caller_instruction,
&executable_accounts,
&accounts,
programs.as_slice(),
Expand Down Expand Up @@ -1303,4 +1327,200 @@ mod tests {
);
}
}

#[test]
fn test_native_invoke() {
#[derive(Debug, Serialize, Deserialize)]
enum MockInstruction {
NoopSuccess,
NoopFail,
ModifyOwned,
ModifyNotOwned,
ModifyReadonly,
}

fn mock_process_instruction(
program_id: &Pubkey,
data: &[u8],
invoke_context: &mut dyn InvokeContext,
) -> Result<(), InstructionError> {
let keyed_accounts = invoke_context.get_keyed_accounts()?;
assert_eq!(*program_id, keyed_accounts[0].owner()?);
assert_ne!(
keyed_accounts[1].owner()?,
*keyed_accounts[0].unsigned_key()
);

if let Ok(instruction) = bincode::deserialize(data) {
match instruction {
MockInstruction::NoopSuccess => (),
MockInstruction::NoopFail => return Err(InstructionError::GenericError),
MockInstruction::ModifyOwned => {
keyed_accounts[0].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
}
MockInstruction::ModifyNotOwned => {
keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
}
MockInstruction::ModifyReadonly => {
keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
}
}
} else {
return Err(InstructionError::InvalidInstructionData);
}
Ok(())
}

let caller_program_id = solana_sdk::pubkey::new_rand();
let callee_program_id = solana_sdk::pubkey::new_rand();

let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
program_account.set_executable(true);
let executable_accounts = vec![(
callee_program_id,
Rc::new(RefCell::new(program_account.clone())),
)];

let owned_account = AccountSharedData::new(42, 1, &callee_program_id);
let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand());
let readonly_account = AccountSharedData::new(168, 1, &caller_program_id);

#[allow(unused_mut)]
let accounts = vec![
(
solana_sdk::pubkey::new_rand(),
Rc::new(RefCell::new(owned_account)),
),
(
solana_sdk::pubkey::new_rand(),
Rc::new(RefCell::new(not_owned_account)),
),
(
solana_sdk::pubkey::new_rand(),
Rc::new(RefCell::new(readonly_account)),
),
(callee_program_id, Rc::new(RefCell::new(program_account))),
];
let programs: Vec<(_, ProcessInstructionWithContext)> =
vec![(callee_program_id, mock_process_instruction)];
let metas = vec![
AccountMeta::new(accounts[0].0, false),
AccountMeta::new(accounts[1].0, false),
AccountMeta::new_readonly(accounts[2].0, false),
];

let caller_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2, 3]);
let callee_instruction = Instruction::new_with_bincode(
callee_program_id,
&MockInstruction::NoopSuccess,
metas.clone(),
);
let message = Message::new(&[callee_instruction.clone()], None);

let feature_set = FeatureSet::all_enabled();
let ancestors = Ancestors::default();
let blockhash = Hash::default();
let fee_calculator = FeeCalculator::default();
let mut invoke_context = ThisInvokeContext::new(
&caller_program_id,
Rent::default(),
&message,
&caller_instruction,
&executable_accounts,
&accounts,
programs.as_slice(),
None,
ComputeBudget::default(),
Rc::new(RefCell::new(MockComputeMeter::default())),
Rc::new(RefCell::new(Executors::default())),
None,
Arc::new(feature_set),
Arc::new(Accounts::default_for_tests()),
&ancestors,
&blockhash,
&fee_calculator,
);

// not owned account modified by the invoker
accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1;
assert_eq!(
InstructionProcessor::native_invoke(
&mut invoke_context,
callee_instruction.clone(),
&[0, 1, 2, 3],
&[]
),
Err(InstructionError::ExternalAccountDataModified)
);
accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0;

// readonly account modified by the invoker
accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1;
assert_eq!(
InstructionProcessor::native_invoke(
&mut invoke_context,
callee_instruction,
&[0, 1, 2, 3],
&[]
),
Err(InstructionError::ReadonlyDataModified)
);
accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0;

// Other test cases
let cases = vec![
(MockInstruction::NoopSuccess, Ok(())),
(
MockInstruction::NoopFail,
Err(InstructionError::GenericError),
),
(MockInstruction::ModifyOwned, Ok(())),
(
MockInstruction::ModifyNotOwned,
Err(InstructionError::ExternalAccountDataModified),
),
(
MockInstruction::ModifyReadonly,
Err(InstructionError::ReadonlyDataModified),
),
];
for case in cases {
let callee_instruction =
Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone());
let message = Message::new(&[callee_instruction.clone()], None);

let ancestors = Ancestors::default();
let blockhash = Hash::default();
let fee_calculator = FeeCalculator::default();
let mut invoke_context = ThisInvokeContext::new(
&caller_program_id,
Rent::default(),
&message,
&caller_instruction,
&executable_accounts,
&accounts,
programs.as_slice(),
None,
ComputeBudget::default(),
Rc::new(RefCell::new(MockComputeMeter::default())),
Rc::new(RefCell::new(Executors::default())),
None,
Arc::new(FeatureSet::all_enabled()),
Arc::new(Accounts::default_for_tests()),
&ancestors,
&blockhash,
&fee_calculator,
);

assert_eq!(
InstructionProcessor::native_invoke(
&mut invoke_context,
callee_instruction,
&[0, 1, 2, 3],
&[]
),
case.1
);
}
}
}

0 comments on commit 84c5619

Please sign in to comment.