diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 37b41d4ecdf..4a71c415911 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -282,13 +282,29 @@ impl<'a, 'ix_data> InvokeContext<'a, 'ix_data> { self.transaction_context.get_instruction_stack_height() } - /// Entrypoint for a cross-program invocation from a builtin program - pub fn native_invoke( + /// Entrypoint for a cross-program invocation from a builtin program. + /// + /// Takes signer seeds and derives PDAs internally via + /// `create_program_address`, mirroring the SBF CPI path. This makes + /// it structurally impossible for a builtin to vouch for a non-PDA + /// address (e.g. a user wallet) as a signer. + pub fn native_invoke_signed( &mut self, instruction: Instruction, - signers: &[Pubkey], + signer_seeds: &[&[&[u8]]], ) -> Result<(), InstructionError> { - self.prepare_next_cpi_instruction(instruction, signers)?; + let caller_program_id = *self + .transaction_context + .get_current_instruction_context()? + .get_program_key()?; + // The conversion from `PubkeyError` to `InstructionError` through + // num-traits is incorrect, but it's the existing behavior. + let signers = signer_seeds + .iter() + .map(|seeds| Pubkey::create_program_address(seeds, &caller_program_id)) + .collect::, solana_pubkey::PubkeyError>>() + .map_err(|e| e as u64)?; + self.prepare_next_cpi_instruction(instruction, &signers)?; let mut compute_units_consumed = 0; self.process_instruction(&mut compute_units_consumed, &mut ExecuteTimings::default())?; Ok(()) @@ -1019,6 +1035,7 @@ mod tests { solana_instruction::Instruction, solana_keypair::Keypair, solana_rent::Rent, + solana_sdk_ids::system_program, solana_signer::Signer, solana_transaction::{Transaction, sanitized::SanitizedTransaction}, solana_transaction_context::MAX_ACCOUNTS_PER_INSTRUCTION, @@ -1117,7 +1134,7 @@ mod tests { assert_eq!(result, Err(InstructionError::UnbalancedInstruction)); result?; invoke_context - .native_invoke(inner_instruction, &[]) + .native_invoke_signed(inner_instruction, &[]) .and(invoke_context.pop())?; } MockInstruction::UnbalancedPop => instruction_context @@ -1339,7 +1356,7 @@ mod tests { let inner_instruction = Instruction::new_with_bincode(callee_program_id, &instruction, metas); let result = invoke_context - .native_invoke(inner_instruction, &[]) + .native_invoke_signed(inner_instruction, &[]) .and(invoke_context.pop()); assert_eq!(result, expected_result); } @@ -1737,4 +1754,151 @@ mod tests { } } } + + // Used for native_invoke_signed tests below. + const TEST_CALLER_PROGRAM_ID: Pubkey = Pubkey::new_from_array([1u8; 32]); + const TEST_CALLEE_PROGRAM_ID: Pubkey = Pubkey::new_from_array([2u8; 32]); + const TEST_WRONG_PROGRAM_ID: Pubkey = Pubkey::new_from_array([3u8; 32]); + const TEST_MOCK_EXTRA_KEY: Pubkey = Pubkey::new_from_array([4u8; 32]); + const TEST_ACCOUNT_KEY: Pubkey = Pubkey::new_from_array([5u8; 32]); + + /// Runs a `native_invoke_signed` call with the standard test setup and returns + /// the result. + /// + /// Same layout for all tests: + /// 0: target account (writable, signer iff `target_is_signer`) + /// 1: caller program (executable) + /// 2: mock extra (satisfies MockBuiltin's 2-account requirement) + /// 3: callee program (executable) + fn run_native_invoke_signed_test( + target_key: Pubkey, + target_is_signer: bool, + inner_instruction: Instruction, + signer_seeds: &[&[&[u8]]], + ) -> Result<(), InstructionError> { + let target_account = AccountSharedData::new(100, 0, &TEST_CALLEE_PROGRAM_ID); + let mock_extra_account = AccountSharedData::new(0, 1, &system_program::id()); + let mut caller_program_account = AccountSharedData::new(1, 1, &native_loader::id()); + caller_program_account.set_executable(true); + let mut callee_program_account = AccountSharedData::new(1, 1, &native_loader::id()); + callee_program_account.set_executable(true); + let transaction_accounts = vec![ + (target_key, target_account), + (TEST_CALLER_PROGRAM_ID, caller_program_account), + (TEST_MOCK_EXTRA_KEY, mock_extra_account), + (TEST_CALLEE_PROGRAM_ID, callee_program_account), + ]; + + with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); + let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); + program_cache_for_tx_batch.replenish( + TEST_CALLEE_PROGRAM_ID, + Arc::new(ProgramCacheEntry::new_builtin(0, 1, MockBuiltin::vm)), + ); + invoke_context.program_cache_for_tx_batch = &mut program_cache_for_tx_batch; + + let instruction_accounts = (0..4) + .map(|i| InstructionAccount::new(i, i == 0 && target_is_signer, i < 2)) + .collect::>(); + invoke_context + .transaction_context + .configure_top_level_instruction_for_tests(1, instruction_accounts, vec![]) + .unwrap(); + invoke_context.push().unwrap(); + + let result = invoke_context.native_invoke_signed(inner_instruction, signer_seeds); + invoke_context.pop().unwrap(); + result + } + + // Valid PDA seeds grant signer privilege to the derived address. + #[test] + fn test_native_invoke_signed_with_valid_pda_signer() { + let (pda_key, bump_seed) = + Pubkey::find_program_address(&[b"seed"], &TEST_CALLER_PROGRAM_ID); + let instruction = Instruction::new_with_bincode( + TEST_CALLEE_PROGRAM_ID, + &MockInstruction::NoopSuccess, + vec![ + AccountMeta::new(pda_key, true), + AccountMeta::new_readonly(TEST_MOCK_EXTRA_KEY, false), + ], + ); + let result = + run_native_invoke_signed_test(pda_key, false, instruction, &[&[b"seed", &[bump_seed]]]); + assert!( + result.is_ok(), + "valid PDA signer should succeed: {result:?}" + ); + } + + // Oversized seeds (>MAX_SEED_LEN) hit `MaxSeedLengthExceeded` + // (discriminant 0) which the broken `as u64` num-traits conversion + // maps to `Custom(0)`. + #[test] + fn test_native_invoke_signed_with_invalid_seeds() { + let instruction = Instruction::new_with_bincode( + TEST_CALLEE_PROGRAM_ID, + &MockInstruction::NoopSuccess, + vec![AccountMeta::new(TEST_ACCOUNT_KEY, true)], + ); + let oversized_seed = [0u8; 33]; + let result = run_native_invoke_signed_test( + TEST_ACCOUNT_KEY, + false, + instruction, + &[&[&oversized_seed]], + ); + assert_eq!(result, Err(InstructionError::Custom(0))); + } + + // CPI marks an account as signer but caller provides no seeds — + // signer privilege escalation. + #[test] + fn test_native_invoke_signed_pda_privilege_escalation_without_seeds() { + let (pda_key, _bump_seed) = + Pubkey::find_program_address(&[b"seed"], &TEST_CALLER_PROGRAM_ID); + let instruction = Instruction::new_with_bincode( + TEST_CALLEE_PROGRAM_ID, + &MockInstruction::NoopSuccess, + vec![AccountMeta::new(pda_key, true)], + ); + let result = run_native_invoke_signed_test(pda_key, false, instruction, &[]); + assert_eq!(result, Err(InstructionError::PrivilegeEscalation)); + } + + // Seeds valid for a different program ID don't grant signer privilege + // because native_invoke_signed derives against the caller's own program ID. + #[test] + fn test_native_invoke_signed_uses_caller_program_id_for_pda() { + let (pda_key, bump_seed) = Pubkey::find_program_address(&[b"seed"], &TEST_WRONG_PROGRAM_ID); + let instruction = Instruction::new_with_bincode( + TEST_CALLEE_PROGRAM_ID, + &MockInstruction::NoopSuccess, + vec![AccountMeta::new(pda_key, true)], + ); + let result = + run_native_invoke_signed_test(pda_key, false, instruction, &[&[b"seed", &[bump_seed]]]); + assert_eq!(result, Err(InstructionError::PrivilegeEscalation)); + } + + // Top-level signer privilege carries through CPI without needing seeds. + #[test] + fn test_native_invoke_signed_top_level_signer_needs_no_seeds() { + let (pda_key, _bump_seed) = + Pubkey::find_program_address(&[b"seed"], &TEST_CALLER_PROGRAM_ID); + let instruction = Instruction::new_with_bincode( + TEST_CALLEE_PROGRAM_ID, + &MockInstruction::NoopSuccess, + vec![ + AccountMeta::new(pda_key, true), + AccountMeta::new_readonly(TEST_MOCK_EXTRA_KEY, false), + ], + ); + let result = run_native_invoke_signed_test(pda_key, true, instruction, &[]); + assert!( + result.is_ok(), + "top-level signer should not need seeds: {result:?}" + ); + } } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index c0bb90a9d79..feec82d134c 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -294,17 +294,8 @@ fn process_loader_upgradeable_instruction( .accounts .push(AccountMeta::new(buffer_key, false)); - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let caller_program_id = instruction_context.get_program_key()?; - // The conversion from `PubkeyError` to `InstructionError` through - // num-traits is incorrect, but it's the existing behavior. - let signers = [[new_program_id.as_ref(), &[bump_seed]]] - .iter() - .map(|seeds| Pubkey::create_program_address(seeds, caller_program_id)) - .collect::, solana_pubkey::PubkeyError>>() - .map_err(|e| e as u64)?; - invoke_context.native_invoke(instruction, signers.as_slice())?; + invoke_context + .native_invoke_signed(instruction, &[&[new_program_id.as_ref(), &[bump_seed]]])?; // Load and verify the program bits let transaction_context = &invoke_context.transaction_context; @@ -877,7 +868,7 @@ fn process_loader_upgradeable_instruction( )), ); } else { - invoke_context.native_invoke( + invoke_context.native_invoke_signed( solana_loader_v4_interface::instruction::set_program_length( &program_address, &provided_authority_address, @@ -887,7 +878,7 @@ fn process_loader_upgradeable_instruction( &[], )?; - invoke_context.native_invoke( + invoke_context.native_invoke_signed( solana_loader_v4_interface::instruction::copy( &program_address, &provided_authority_address, @@ -899,7 +890,7 @@ fn process_loader_upgradeable_instruction( &[], )?; - invoke_context.native_invoke( + invoke_context.native_invoke_signed( solana_loader_v4_interface::instruction::deploy( &program_address, &provided_authority_address, @@ -909,7 +900,7 @@ fn process_loader_upgradeable_instruction( if let Some(upgrade_authority_address) = upgrade_authority_address { if migration_authority::check_id(&provided_authority_address) { - invoke_context.native_invoke( + invoke_context.native_invoke_signed( solana_loader_v4_interface::instruction::transfer_authority( &program_address, &provided_authority_address, @@ -919,7 +910,7 @@ fn process_loader_upgradeable_instruction( )?; } } else { - invoke_context.native_invoke( + invoke_context.native_invoke_signed( solana_loader_v4_interface::instruction::finalize( &program_address, &provided_authority_address, @@ -1068,7 +1059,7 @@ fn common_extend_program( min_balance.saturating_sub(balance) }; - // Borrowed accounts need to be dropped before native_invoke + // Borrowed accounts need to be dropped before native_invoke_signed drop(programdata_account); // Dereference the program ID to prevent overlapping mutable/immutable borrow of invoke context @@ -1077,7 +1068,7 @@ fn common_extend_program( let payer_key = *instruction_context.get_key_of_instruction_account(optional_payer_account_index)?; - invoke_context.native_invoke( + invoke_context.native_invoke_signed( system_instruction::transfer(&payer_key, &programdata_key, required_payment), &[], )?; diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index e12d4883a67..0595e6ef85c 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1014,7 +1014,7 @@ pub fn deposit_delegator_rewards( }?; // CPI to System: Transfer from sender to vote account. - invoke_context.native_invoke( + invoke_context.native_invoke_signed( system_instruction::transfer(&source_address, &vote_address, deposit), &[], )?; diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 3ec6cc23064..96327444a15 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -4703,7 +4703,7 @@ pub mod tests { let to_pubkey = *instruction_context.get_key_of_instruction_account(1)?; let owner_pubkey = *instruction_context.get_key_of_instruction_account(2)?; - invoke_context.native_invoke( + invoke_context.native_invoke_signed( system_instruction::create_account( &from_pubkey, &to_pubkey, diff --git a/runtime/src/bank/builtins/core_bpf_migration/mod.rs b/runtime/src/bank/builtins/core_bpf_migration/mod.rs index 923d525deab..ad1480ba4e9 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/mod.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/mod.rs @@ -1261,7 +1261,7 @@ pub(crate) mod tests { let instruction = Instruction::new_with_bytes(*target_program_id, &[], Vec::new()); - invoke_context.native_invoke(instruction, &[]) + invoke_context.native_invoke_signed(instruction, &[]) }); }