From 77572a7c53fbcebec3b43e7c3d43a6904c845bab Mon Sep 17 00:00:00 2001 From: Jack May Date: Fri, 22 Jan 2021 15:28:01 -0800 Subject: [PATCH] Track account writable deescalation (#14626) --- program-test/src/lib.rs | 8 ++ programs/bpf/c/src/invoke/invoke.c | 36 +++++- programs/bpf/c/src/invoked/instruction.h | 1 + programs/bpf/c/src/invoked/invoked.c | 14 +++ programs/bpf/rust/invoke/src/lib.rs | 32 +++++ programs/bpf/rust/invoked/src/instruction.rs | 1 + programs/bpf/rust/invoked/src/processor.rs | 6 + programs/bpf/tests/programs.rs | 9 ++ programs/bpf_loader/src/syscalls.rs | 25 +++- runtime/benches/message_processor.rs | 13 +- runtime/src/message_processor.rs | 118 +++++++++++++------ scripts/build-downstream-projects.sh | 16 +-- sdk/src/feature_set.rs | 21 ++-- sdk/src/process_instruction.rs | 2 + 14 files changed, 246 insertions(+), 56 deletions(-) diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 7faf6d5396795e..9bb49c5188f622 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -210,6 +210,13 @@ impl program_stubs::SyscallStubs for SyscallStubs { let program_id_index = message.instructions[0].program_id_index as usize; let program_id = message.account_keys[program_id_index]; let program_account_info = &account_infos[program_id_index]; + // TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet + let caller_privileges = message + .account_keys + .iter() + .enumerate() + .map(|(i, _)| message.is_writable(i)) + .collect::>(); stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth()); @@ -268,6 +275,7 @@ impl program_stubs::SyscallStubs for SyscallStubs { &message, &executables, &accounts, + &caller_privileges, invoke_context, ) .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index 0fa0ab13979fea..8b476bce5690f6 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -17,6 +17,7 @@ static const uint8_t TEST_INSTRUCTION_META_TOO_LARGE = 10; static const uint8_t TEST_RETURN_ERROR = 11; static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER = 12; static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 13; +static const uint8_t TEST_WRITE_DEESCALATION = 14; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -251,6 +252,26 @@ extern uint64_t entrypoint(const uint8_t *input) { for (int i = 0; i < accounts[INVOKED_ARGUMENT_INDEX].data_len; i++) { sol_assert(accounts[INVOKED_ARGUMENT_INDEX].data[i] == i); } + + sol_log("Verify data write before ro cpi call"); + { + for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) { + accounts[ARGUMENT_INDEX].data[i] = 0; + } + + SolAccountMeta arguments[] = { + {accounts[ARGUMENT_INDEX].key, false, false}}; + uint8_t data[] = {VERIFY_PRIVILEGE_DEESCALATION}; + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_assert(SUCCESS == + sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts))); + + for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) { + sol_assert(accounts[ARGUMENT_INDEX].data[i] == 0); + } + } break; } case TEST_PRIVILEGE_ESCALATION_SIGNER: { @@ -443,7 +464,8 @@ extern uint64_t entrypoint(const uint8_t *input) { break; } case TEST_RETURN_ERROR: { - SolAccountMeta arguments[] = {{accounts[ARGUMENT_INDEX].key, true, true}}; + sol_log("Test return error"); + SolAccountMeta arguments[] = {{accounts[ARGUMENT_INDEX].key, false, true}}; uint8_t data[] = {RETURN_ERROR}; const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, arguments, SOL_ARRAY_SIZE(arguments), @@ -484,6 +506,18 @@ extern uint64_t entrypoint(const uint8_t *input) { break; } + case TEST_WRITE_DEESCALATION: { + sol_log("Test writable deescalation"); + + SolAccountMeta arguments[] = { + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}}; + uint8_t data[] = {WRITE_ACCOUNT, 10}; + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); + break; + } default: sol_panic(); } diff --git a/programs/bpf/c/src/invoked/instruction.h b/programs/bpf/c/src/invoked/instruction.h index 4863a70e5b742a..5f3e3998d3cbcd 100644 --- a/programs/bpf/c/src/invoked/instruction.h +++ b/programs/bpf/c/src/invoked/instruction.h @@ -15,3 +15,4 @@ const uint8_t RETURN_OK = 7; const uint8_t VERIFY_PRIVILEGE_DEESCALATION = 8; const uint8_t VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER = 9; const uint8_t VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 10; +const uint8_t WRITE_ACCOUNT = 11; diff --git a/programs/bpf/c/src/invoked/invoked.c b/programs/bpf/c/src/invoked/invoked.c index 4bde1f234142bf..f554ca9a2a2699 100644 --- a/programs/bpf/c/src/invoked/invoked.c +++ b/programs/bpf/c/src/invoked/invoked.c @@ -158,6 +158,7 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(accounts[ARGUMENT_INDEX].is_writable); break; } + case VERIFY_PRIVILEGE_ESCALATION: { sol_log("Should never get here!"); break; @@ -188,6 +189,7 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts))); break; } + case VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: { sol_log("verify privilege deescalation escalation writable"); static const int INVOKED_PROGRAM_INDEX = 0; @@ -245,6 +247,18 @@ extern uint64_t entrypoint(const uint8_t *input) { } break; } + + case WRITE_ACCOUNT: { + sol_log("write account"); + static const int INVOKED_ARGUMENT_INDEX = 0; + sol_assert(sol_deserialize(input, ¶ms, 1)); + + for (int i = 0; i < params.data[1]; i++) { + accounts[INVOKED_ARGUMENT_INDEX].data[i] = params.data[1]; + } + break; + } + default: return ERROR_INVALID_INSTRUCTION_DATA; } diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index d6594d69cedbd5..e5b119b794f770 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -29,6 +29,7 @@ const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10; const TEST_RETURN_ERROR: u8 = 11; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 12; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; +const TEST_WRITE_DEESCALATION: u8 = 14; // const MINT_INDEX: usize = 0; const ARGUMENT_INDEX: usize = 1; @@ -331,6 +332,28 @@ fn process_instruction( assert_eq!(data[i as usize], i); } } + + msg!("Verify data write before cpi call with deescalated writable"); + { + { + let mut data = accounts[ARGUMENT_INDEX].try_borrow_mut_data()?; + for i in 0..100 { + data[i as usize] = 42; + } + } + + let invoked_instruction = create_instruction( + *accounts[INVOKED_PROGRAM_INDEX].key, + &[(accounts[ARGUMENT_INDEX].key, false, false)], + vec![VERIFY_PRIVILEGE_DEESCALATION], + ); + invoke(&invoked_instruction, accounts)?; + + let data = accounts[ARGUMENT_INDEX].try_borrow_data()?; + for i in 0..100 { + assert_eq!(data[i as usize], 42); + } + } } TEST_PRIVILEGE_ESCALATION_SIGNER => { msg!("Test privilege escalation signer"); @@ -534,6 +557,15 @@ fn process_instruction( ); invoke(&invoked_instruction, accounts)?; } + TEST_WRITE_DEESCALATION => { + msg!("Test writable deescalation"); + let instruction = create_instruction( + *accounts[INVOKED_PROGRAM_INDEX].key, + &[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)], + vec![WRITE_ACCOUNT, 10], + ); + let _ = invoke(&instruction, accounts); + } _ => panic!(), } diff --git a/programs/bpf/rust/invoked/src/instruction.rs b/programs/bpf/rust/invoked/src/instruction.rs index 328dcd3c83a2d1..770ea086190a5a 100644 --- a/programs/bpf/rust/invoked/src/instruction.rs +++ b/programs/bpf/rust/invoked/src/instruction.rs @@ -16,6 +16,7 @@ pub const RETURN_OK: u8 = 7; pub const VERIFY_PRIVILEGE_DEESCALATION: u8 = 8; pub const VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 9; pub const VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 10; +pub const WRITE_ACCOUNT: u8 = 11; pub fn create_instruction( program_id: Pubkey, diff --git a/programs/bpf/rust/invoked/src/processor.rs b/programs/bpf/rust/invoked/src/processor.rs index e17d4b6b34df72..1b01236bd83db9 100644 --- a/programs/bpf/rust/invoked/src/processor.rs +++ b/programs/bpf/rust/invoked/src/processor.rs @@ -229,6 +229,12 @@ fn process_instruction( } } } + WRITE_ACCOUNT => { + msg!("write account"); + for i in 0..instruction_data[1] { + accounts[0].data.borrow_mut()[i as usize] = instruction_data[1]; + } + } _ => panic!(), } diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 92a47f667cd398..105f11a5e95776 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -697,6 +697,7 @@ fn test_program_bpf_invoke_sanity() { const TEST_RETURN_ERROR: u8 = 11; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_SIGNER: u8 = 12; const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; + const TEST_WRITE_DEESCALATION: u8 = 14; #[allow(dead_code)] #[derive(Debug)] @@ -813,6 +814,7 @@ fn test_program_bpf_invoke_sanity() { invoked_program_id.clone(), invoked_program_id.clone(), invoked_program_id.clone(), + invoked_program_id.clone(), ], Languages::Rust => vec![ solana_sdk::system_program::id(), @@ -830,6 +832,7 @@ fn test_program_bpf_invoke_sanity() { invoked_program_id.clone(), invoked_program_id.clone(), invoked_program_id.clone(), + invoked_program_id.clone(), ], }; assert_eq!(invoked_programs.len(), expected_invoked_programs.len()); @@ -931,6 +934,12 @@ fn test_program_bpf_invoke_sanity() { &[invoked_program_id.clone()], ); + do_invoke_failure_test_local( + TEST_WRITE_DEESCALATION, + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified), + &[invoked_program_id.clone()], + ); + // Check resulting state assert_eq!(43, bank.get_balance(&derived_key1)); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 2def312bae1d7a..7b073a3112945a 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -1528,7 +1528,14 @@ fn call<'a>( signers_seeds_len: u64, memory_mapping: &MemoryMapping, ) -> Result> { - let (message, executables, accounts, account_refs, abort_on_all_cpi_failures) = { + let ( + message, + executables, + accounts, + account_refs, + caller_privileges, + abort_on_all_cpi_failures, + ) = { let invoke_context = syscall.get_context()?; invoke_context @@ -1555,6 +1562,20 @@ fn call<'a>( let (message, callee_program_id, callee_program_id_index) = MessageProcessor::create_message(&instruction, &keyed_account_refs, &signers) .map_err(SyscallError::InstructionError)?; + let caller_privileges = message + .account_keys + .iter() + .map(|key| { + if let Some(keyed_account) = keyed_account_refs + .iter() + .find(|keyed_account| key == keyed_account.unsigned_key()) + { + keyed_account.is_writable() + } else { + false + } + }) + .collect::>(); if invoke_context.is_feature_active(&limit_cpi_loader_invoke::id()) { check_authorized_program(&callee_program_id, &instruction.data)?; } @@ -1590,6 +1611,7 @@ fn call<'a>( executables, accounts, account_refs, + caller_privileges, invoke_context.is_feature_active(&abort_on_all_cpi_failures::id()), ) }; @@ -1601,6 +1623,7 @@ fn call<'a>( &message, &executables, &accounts, + &caller_privileges, *(&mut *(syscall.get_context_mut()?)), ) { Ok(()) => (), diff --git a/runtime/benches/message_processor.rs b/runtime/benches/message_processor.rs index bf4d16d256e562..638fbf08581b0b 100644 --- a/runtime/benches/message_processor.rs +++ b/runtime/benches/message_processor.rs @@ -16,15 +16,18 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) { let pre = PreAccount::new( &pubkey::new_rand(), &Account::new(0, BUFSIZE, &owner), - true, false, ); let post = Account::new(0, BUFSIZE, &owner); - assert_eq!(pre.verify(&owner, &Rent::default(), &post), Ok(())); + assert_eq!( + pre.verify(&owner, Some(false), &Rent::default(), &post), + Ok(()) + ); // this one should be faster bencher.iter(|| { - pre.verify(&owner, &Rent::default(), &post).unwrap(); + pre.verify(&owner, Some(false), &Rent::default(), &post) + .unwrap(); }); let summary = bencher.bench(|_bencher| {}).unwrap(); info!("data no change by owner: {} ns/iter", summary.median); @@ -38,11 +41,11 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) { let pre = PreAccount::new( &pubkey::new_rand(), &Account::new(0, BUFSIZE, &owner), - true, false, ); bencher.iter(|| { - pre.verify(&non_owner, &Rent::default(), &post).unwrap(); + pre.verify(&non_owner, Some(false), &Rent::default(), &post) + .unwrap(); }); let summary = bencher.bench(|_bencher| {}).unwrap(); info!("data no change by non owner: {} ns/iter", summary.median); diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 9cbf2c5ec479dd..a661bba47b5d7a 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -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, } 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, 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>], + 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::, solana_sdk::pubkey::PubkeyError>>()?; + let mut caller_privileges = keyed_accounts + .iter() + .map(|keyed_account| keyed_account.is_writable()) + .collect::>(); + 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)], accounts: &[Rc>], + 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>], 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(¬_owned_key, ¬_owned_account, false, true); + let not_owned_preaccount = PreAccount::new(¬_owned_key, ¬_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::>(); 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::>(); assert_eq!( MessageProcessor::process_cross_program_instruction( &message, &executable_accounts, &accounts, + &caller_privileges, &mut invoke_context, ), case.1 diff --git a/scripts/build-downstream-projects.sh b/scripts/build-downstream-projects.sh index eab301858bbe54..2a2188bb626864 100755 --- a/scripts/build-downstream-projects.sh +++ b/scripts/build-downstream-projects.sh @@ -58,11 +58,11 @@ example_helloworld() { spl() { ( set -x - rm -rf spl - git clone https://github.com/solana-labs/solana-program-library.git spl + # rm -rf spl + # git clone https://github.com/solana-labs/solana-program-library.git spl cd spl - ./patch.crates-io.sh "$solana_dir" + # ./patch.crates-io.sh "$solana_dir" $cargo build @@ -71,9 +71,9 @@ spl() { #$cargo test #$cargo_test_bpf - $cargo_test_bpf --manifest-path token/program/Cargo.toml - $cargo_test_bpf --manifest-path associated-token-account/program/Cargo.toml - $cargo_test_bpf --manifest-path feature-proposal/program/Cargo.toml + # $cargo_test_bpf --manifest-path token/program/Cargo.toml + # $cargo_test_bpf --manifest-path associated-token-account/program/Cargo.toml + $cargo_test_bpf --manifest-path feature-proposal/program/Cargo.toml -- --nocapture ) } @@ -102,6 +102,6 @@ serum_dex() { } -_ example_helloworld +# _ example_helloworld _ spl -_ serum_dex +# _ serum_dex diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 65b60f70ac6aba..e2715c213f3216 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -166,6 +166,10 @@ pub mod prevent_upgrade_and_invoke { solana_sdk::declare_id!("BiNjYd8jCYDgAwMqP91uwZs6skWpuHtKrZbckuKESs8N"); } +pub mod track_writable_deescalation { + solana_sdk::declare_id!("HVPSxqskEtRLRT2ZeEMmkmt9FWqoFX4vrN6f5VaadLED"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -197,15 +201,16 @@ lazy_static! { (try_find_program_address_syscall_enabled::id(), "add try_find_program_address syscall"), (warp_timestamp::id(), "warp timestamp to current, adjust bounding to 50% #14210 & #14531"), (stake_program_v3::id(), "solana_stake_program v3"), - (max_cpi_instruction_size_ipv6_mtu::id(), "Max cross-program invocation size 1280"), - (limit_cpi_loader_invoke::id(), "Loader not authorized via CPI"), - (use_loaded_program_accounts::id(), "Use loaded program accounts"), - (abort_on_all_cpi_failures::id(), "Abort on all CPI failures"), - (use_loaded_executables::id(), "Use loaded executable accounts"), + (max_cpi_instruction_size_ipv6_mtu::id(), "max cross-program invocation size 1280"), + (limit_cpi_loader_invoke::id(), "loader not authorized via CPI"), + (use_loaded_program_accounts::id(), "use loaded program accounts"), + (abort_on_all_cpi_failures::id(), "abort on all CPI failures"), + (use_loaded_executables::id(), "use loaded executable accounts"), (turbine_retransmit_peers_patch::id(), "turbine retransmit peers patch #14631"), - (prevent_upgrade_and_invoke::id(), "Prevent upgrade and invoke in same tx batch"), - (full_inflation::candidate_example::vote::id(), "Community vote allowing candidate_example to enable full inflation"), - (full_inflation::candidate_example::enable::id(), "Full inflation enabled by candidate_example"), + (prevent_upgrade_and_invoke::id(), "prevent upgrade and invoke in same tx batch"), + (full_inflation::candidate_example::vote::id(), "community vote allowing candidate_example to enable full inflation"), + (full_inflation::candidate_example::enable::id(), "full inflation enabled by candidate_example"), + (track_writable_deescalation::id(), "Track account writable deescalation"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/process_instruction.rs b/sdk/src/process_instruction.rs index 454fc3758c5a94..b0e93fd8c9d830 100644 --- a/sdk/src/process_instruction.rs +++ b/sdk/src/process_instruction.rs @@ -41,6 +41,7 @@ pub trait InvokeContext { message: &Message, instruction: &CompiledInstruction, accounts: &[Rc>], + caller_pivileges: Option<&[bool]>, ) -> Result<(), InstructionError>; /// Get the program ID of the currently executing program fn get_caller(&self) -> Result<&Pubkey, InstructionError>; @@ -340,6 +341,7 @@ impl InvokeContext for MockInvokeContext { _message: &Message, _instruction: &CompiledInstruction, _accounts: &[Rc>], + _caller_pivileges: Option<&[bool]>, ) -> Result<(), InstructionError> { Ok(()) }