Fix native invoke writable privileges (#19750)
* Fix native invoke writable privileges * build downstream spl bpf programs for tests
This commit is contained in:
Jack May
GitHub
parent
910f241c3f
commit
00d7981f64
|
|
@ -4,7 +4,7 @@ use solana_sdk::{
|
|||
account::{AccountSharedData, ReadableAccount, WritableAccount},
|
||||
account_utils::StateMut,
|
||||
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
|
||||
feature_set::demote_program_write_locks,
|
||||
feature_set::{demote_program_write_locks, fix_write_privs},
|
||||
ic_logger_msg, ic_msg,
|
||||
instruction::{CompiledInstruction, Instruction, InstructionError},
|
||||
keyed_account::{keyed_account_at_index, KeyedAccount},
|
||||
|
|
@ -474,38 +474,64 @@ 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()?;
|
||||
'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];
|
||||
if account_key == keyed_account.unsigned_key() {
|
||||
accounts.push((*account_key, Rc::new(keyed_account.account.clone())));
|
||||
keyed_account_indices_reordered.push(*keyed_account_index);
|
||||
continue 'root;
|
||||
let (message, callee_program_id, _) = Self::create_message(
|
||||
&instruction,
|
||||
&callee_keyed_accounts,
|
||||
signers,
|
||||
&invoke_context,
|
||||
)?;
|
||||
let mut keyed_account_indices_reordered =
|
||||
Vec::with_capacity(message.account_keys.len());
|
||||
let mut accounts = Vec::with_capacity(message.account_keys.len());
|
||||
let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len());
|
||||
|
||||
// Translate and verify caller's data
|
||||
if invoke_context.is_feature_active(&fix_write_privs::id()) {
|
||||
'root: for account_key in message.account_keys.iter() {
|
||||
for keyed_account_index in keyed_account_indices {
|
||||
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;
|
||||
}
|
||||
}
|
||||
ic_msg!(
|
||||
invoke_context,
|
||||
"Instruction references an unknown account {}",
|
||||
account_key
|
||||
);
|
||||
return Err(InstructionError::MissingAccount);
|
||||
}
|
||||
} else {
|
||||
let keyed_accounts = invoke_context.get_keyed_accounts()?;
|
||||
for index in keyed_account_indices.iter() {
|
||||
caller_write_privileges.push(keyed_accounts[*index].is_writable());
|
||||
}
|
||||
caller_write_privileges.insert(0, false);
|
||||
let keyed_accounts = invoke_context.get_keyed_accounts()?;
|
||||
'root2: for account_key in message.account_keys.iter() {
|
||||
for keyed_account_index in keyed_account_indices {
|
||||
let keyed_account = &keyed_accounts[*keyed_account_index];
|
||||
if account_key == keyed_account.unsigned_key() {
|
||||
accounts.push((*account_key, Rc::new(keyed_account.account.clone())));
|
||||
keyed_account_indices_reordered.push(*keyed_account_index);
|
||||
continue 'root2;
|
||||
}
|
||||
}
|
||||
ic_msg!(
|
||||
invoke_context,
|
||||
"Instruction references an unknown account {}",
|
||||
account_key
|
||||
);
|
||||
return Err(InstructionError::MissingAccount);
|
||||
}
|
||||
ic_msg!(
|
||||
invoke_context,
|
||||
"Instruction references an unknown account {}",
|
||||
account_key
|
||||
);
|
||||
return Err(InstructionError::MissingAccount);
|
||||
}
|
||||
|
||||
// Process instruction
|
||||
|
|
|
|||
|
|
@ -1162,6 +1162,7 @@ mod tests {
|
|||
NoopFail,
|
||||
ModifyOwned,
|
||||
ModifyNotOwned,
|
||||
ModifyReadonly,
|
||||
}
|
||||
|
||||
fn mock_process_instruction(
|
||||
|
|
@ -1186,6 +1187,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);
|
||||
|
|
@ -1196,11 +1200,11 @@ mod tests {
|
|||
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 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);
|
||||
let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
|
||||
program_account.set_executable(true);
|
||||
|
||||
#[allow(unused_mut)]
|
||||
let accounts = vec![
|
||||
|
|
@ -1213,26 +1217,29 @@ mod tests {
|
|||
Rc::new(RefCell::new(not_owned_account)),
|
||||
),
|
||||
(
|
||||
callee_program_id,
|
||||
Rc::new(RefCell::new(program_account.clone())),
|
||||
solana_sdk::pubkey::new_rand(),
|
||||
Rc::new(RefCell::new(readonly_account)),
|
||||
),
|
||||
(callee_program_id, Rc::new(RefCell::new(program_account))),
|
||||
];
|
||||
let program_indices = vec![2];
|
||||
let program_indices = vec![3];
|
||||
|
||||
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());
|
||||
|
||||
|
|
@ -1243,7 +1250,7 @@ mod tests {
|
|||
&caller_program_id,
|
||||
Rent::default(),
|
||||
&message,
|
||||
&compiled_instruction,
|
||||
&caller_instruction,
|
||||
&program_indices,
|
||||
&accounts,
|
||||
programs.as_slice(),
|
||||
|
|
@ -1280,6 +1287,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,
|
||||
&program_indices,
|
||||
&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(())),
|
||||
(
|
||||
|
|
@ -1294,9 +1315,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();
|
||||
|
|
@ -1305,7 +1326,7 @@ mod tests {
|
|||
&caller_program_id,
|
||||
Rent::default(),
|
||||
&message,
|
||||
&compiled_instruction,
|
||||
&caller_instruction,
|
||||
&program_indices,
|
||||
&accounts,
|
||||
programs.as_slice(),
|
||||
|
|
@ -1340,4 +1361,198 @@ 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 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);
|
||||
let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
|
||||
program_account.set_executable(true);
|
||||
|
||||
#[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 program_indices = vec![3];
|
||||
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,
|
||||
&program_indices,
|
||||
&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,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// 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,
|
||||
&program_indices,
|
||||
&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,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
InstructionProcessor::native_invoke(
|
||||
&mut invoke_context,
|
||||
callee_instruction,
|
||||
&[0, 1, 2, 3],
|
||||
&[]
|
||||
),
|
||||
case.1
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ spl() {
|
|||
|
||||
$cargo build
|
||||
$cargo test
|
||||
$cargo_build_bpf
|
||||
$cargo_test_bpf
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -211,6 +211,10 @@ pub mod return_data_syscall_enabled {
|
|||
solana_sdk::declare_id!("BJVXq6NdLC7jCDGjfqJv7M1XHD4Y13VrpDqRF2U7UBcC");
|
||||
}
|
||||
|
||||
pub mod fix_write_privs {
|
||||
solana_sdk::declare_id!("7Tr5C1tdcCeBVD8jxtHYnvjL1DGdFboYBHCJkEFdenBb");
|
||||
}
|
||||
|
||||
lazy_static! {
|
||||
/// Map of feature identifiers to user-visible description
|
||||
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
|
||||
|
|
@ -258,7 +262,8 @@ lazy_static! {
|
|||
(ed25519_program_enabled::id(), "enable builtin ed25519 signature verify program"),
|
||||
(allow_native_ids::id(), "allow native program ids in program derived addresses"),
|
||||
(check_seed_length::id(), "Check program address seed lengths"),
|
||||
(return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall")
|
||||
(return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall"),
|
||||
(fix_write_privs::id(), "fix native invoke write privileges"),
|
||||
/*************** ADD NEW FEATURES HERE ***************/
|
||||
]
|
||||
.iter()
|
||||
|
|
|
|||
Loading…
Reference in New Issue