From 8fee9a2e1ab17b1fe14604568a2bd5edd9e6d029 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 28 Sep 2021 23:25:08 -0700 Subject: [PATCH] Dont call precompiled programs (#19930) --- Cargo.lock | 25 +- Cargo.toml | 2 - program-runtime/src/instruction_processor.rs | 19 +- program-test/src/lib.rs | 4 +- programs/bpf/Cargo.lock | 16 - programs/bpf/benches/bpf_loader.rs | 2 +- programs/bpf/c/src/invoke/invoke.c | 19 +- programs/bpf/rust/invoke/src/lib.rs | 12 +- programs/bpf/tests/programs.rs | 73 ++-- programs/bpf_loader/src/lib.rs | 2 +- programs/bpf_loader/src/syscalls.rs | 20 +- programs/ed25519/Cargo.toml | 25 -- programs/ed25519/src/lib.rs | 55 --- programs/failure/tests/failure.rs | 2 +- programs/ownable/src/ownable_processor.rs | 2 +- programs/secp256k1/Cargo.toml | 26 -- programs/secp256k1/src/lib.rs | 69 ---- runtime/Cargo.toml | 4 +- runtime/benches/bank.rs | 4 +- runtime/src/bank.rs | 368 ++++++++++++++----- runtime/src/builtins.rs | 27 +- runtime/src/message_processor.rs | 83 ++++- runtime/tests/noop.rs | 2 +- sdk/src/ed25519_instruction.rs | 39 ++ sdk/src/feature_set.rs | 7 + sdk/src/precompiles.rs | 30 +- sdk/src/secp256k1_instruction.rs | 53 +++ 27 files changed, 604 insertions(+), 386 deletions(-) delete mode 100644 programs/ed25519/Cargo.toml delete mode 100644 programs/ed25519/src/lib.rs delete mode 100644 programs/secp256k1/Cargo.toml delete mode 100644 programs/secp256k1/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index f0d9ec5eefcfbd..4cb943aac86295 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4563,16 +4563,6 @@ dependencies = [ "solana-sdk", ] -[[package]] -name = "solana-ed25519-program" -version = "1.8.0" -dependencies = [ - "ed25519-dalek", - "rand 0.7.3", - "solana-logger 1.8.0", - "solana-sdk", -] - [[package]] name = "solana-entry" version = "1.8.0" @@ -5365,10 +5355,12 @@ dependencies = [ "crossbeam-channel", "dashmap", "dir-diff", + "ed25519-dalek", "flate2", "fnv", "itertools 0.10.1", "lazy_static", + "libsecp256k1 0.6.0", "log 0.4.14", "memmap2 0.5.0", "num_cpus", @@ -5382,7 +5374,6 @@ dependencies = [ "solana-bucket-map", "solana-compute-budget-program", "solana-config-program", - "solana-ed25519-program", "solana-frozen-abi 1.8.0", "solana-frozen-abi-macro 1.8.0", "solana-logger 1.8.0", @@ -5391,7 +5382,6 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", - "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -5483,17 +5473,6 @@ dependencies = [ "syn 1.0.67", ] -[[package]] -name = "solana-secp256k1-program" -version = "1.8.0" -dependencies = [ - "bincode", - "libsecp256k1 0.6.0", - "rand 0.7.3", - "solana-logger 1.8.0", - "solana-sdk", -] - [[package]] name = "solana-send-transaction-service" version = "1.8.0" diff --git a/Cargo.toml b/Cargo.toml index 7f49ba685726d9..434e97e89a7a57 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,11 +46,9 @@ members = [ "programs/bpf_loader", "programs/compute-budget", "programs/config", - "programs/ed25519", "programs/failure", "programs/noop", "programs/ownable", - "programs/secp256k1", "programs/stake", "programs/vote", "rbpf-cli", diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs index edc8ec8493a553..97a086fa18acef 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -343,12 +343,19 @@ impl InstructionProcessor { /// Add a static entrypoint to intercept instructions before the dynamic loader. pub fn add_program( &mut self, - program_id: Pubkey, + program_id: &Pubkey, process_instruction: ProcessInstructionWithContext, ) { - match self.programs.iter_mut().find(|(key, _)| program_id == *key) { + match self.programs.iter_mut().find(|(key, _)| program_id == key) { Some((_, processor)) => *processor = process_instruction, - None => self.programs.push((program_id, process_instruction)), + None => self.programs.push((*program_id, process_instruction)), + } + } + + /// Remove a program + pub fn remove_program(&mut self, program_id: &Pubkey) { + if let Some(position) = self.programs.iter().position(|(key, _)| program_id == key) { + self.programs.remove(position); } } @@ -608,7 +615,7 @@ impl InstructionProcessor { let mut instruction_processor = InstructionProcessor::default(); for (program_id, process_instruction) in invoke_context.get_programs().iter() { - instruction_processor.add_program(*program_id, *process_instruction); + instruction_processor.add_program(program_id, *process_instruction); } let mut result = instruction_processor.process_instruction( @@ -1090,8 +1097,8 @@ mod tests { Ok(()) } let program_id = solana_sdk::pubkey::new_rand(); - instruction_processor.add_program(program_id, mock_process_instruction); - instruction_processor.add_program(program_id, mock_ix_processor); + instruction_processor.add_program(&program_id, mock_process_instruction); + instruction_processor.add_program(&program_id, mock_ix_processor); assert!(!format!("{:?}", instruction_processor).is_empty()); } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 69b97f5ecf6010..4219e63e3f847f 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -774,7 +774,7 @@ impl ProgramTest { // Add loaders macro_rules! add_builtin { ($b:expr) => { - bank.add_builtin(&$b.0, $b.1, $b.2) + bank.add_builtin(&$b.0, &$b.1, $b.2) }; } add_builtin!(solana_bpf_loader_deprecated_program!()); @@ -795,7 +795,7 @@ impl ProgramTest { for builtin in self.builtins.iter() { bank.add_builtin( &builtin.name, - builtin.id, + &builtin.id, builtin.process_instruction_with_context, ); } diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 48c04b77d9987f..4952d3afad1e4f 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -2952,13 +2952,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "solana-ed25519-program" -version = "1.8.0" -dependencies = [ - "solana-sdk", -] - [[package]] name = "solana-faucet" version = "1.8.0" @@ -3266,7 +3259,6 @@ dependencies = [ "solana-bucket-map", "solana-compute-budget-program", "solana-config-program", - "solana-ed25519-program", "solana-frozen-abi 1.8.0", "solana-frozen-abi-macro 1.8.0", "solana-logger 1.8.0", @@ -3275,7 +3267,6 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", - "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -3357,13 +3348,6 @@ dependencies = [ "syn 1.0.67", ] -[[package]] -name = "solana-secp256k1-program" -version = "1.8.0" -dependencies = [ - "solana-sdk", -] - [[package]] name = "solana-send-transaction-service" version = "1.8.0" diff --git a/programs/bpf/benches/bpf_loader.rs b/programs/bpf/benches/bpf_loader.rs index c1f2054a43ea6b..5742013a3d67e4 100644 --- a/programs/bpf/benches/bpf_loader.rs +++ b/programs/bpf/benches/bpf_loader.rs @@ -172,7 +172,7 @@ fn bench_program_execute_noop(bencher: &mut Bencher) { } = create_genesis_config(50); let mut bank = Bank::new_for_benches(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index c7807b4f280fa3..9f62ca3cef216e 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -26,8 +26,9 @@ static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 13; static const uint8_t TEST_WRITABLE_DEESCALATION_WRITABLE = 14; static const uint8_t TEST_NESTED_INVOKE_TOO_DEEP = 15; static const uint8_t TEST_EXECUTABLE_LAMPORTS = 16; -static const uint8_t ADD_LAMPORTS = 17; -static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 18; +static const uint8_t TEST_CALL_PRECOMPILE = 17; +static const uint8_t ADD_LAMPORTS = 18; +static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 19; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -40,6 +41,8 @@ static const int DERIVED_KEY2_INDEX = 7; static const int DERIVED_KEY3_INDEX = 8; static const int SYSTEM_PROGRAM_INDEX = 9; static const int FROM_INDEX = 10; +static const int ED25519_PROGRAM_INDEX = 11; +static const int INVOKE_PROGRAM_INDEX = 12; uint64_t do_nested_invokes(uint64_t num_nested_invokes, SolAccountInfo *accounts, uint64_t num_accounts) { @@ -73,7 +76,7 @@ uint64_t do_nested_invokes(uint64_t num_nested_invokes, extern uint64_t entrypoint(const uint8_t *input) { sol_log("Invoke C program"); - SolAccountInfo accounts[12]; + SolAccountInfo accounts[13]; SolParameters params = (SolParameters){.ka = accounts}; if (!sol_deserialize(input, ¶ms, SOL_ARRAY_SIZE(accounts))) { @@ -588,6 +591,16 @@ extern uint64_t entrypoint(const uint8_t *input) { *accounts[ARGUMENT_INDEX].lamports += 1; break; } + case TEST_CALL_PRECOMPILE: { + sol_log("Test calling precompile from cpi"); + SolAccountMeta arguments[] = {}; + uint8_t data[] = {}; + const SolInstruction instruction = {accounts[ED25519_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); + break; + } case ADD_LAMPORTS: { *accounts[0].lamports += 1; break; diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index 31171c7b0316f3..55286a8efcebfd 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -9,6 +9,7 @@ use solana_program::{ account_info::AccountInfo, entrypoint, entrypoint::{ProgramResult, MAX_PERMITTED_DATA_INCREASE}, + instruction::Instruction, msg, program::{get_return_data, invoke, invoke_signed, set_return_data}, program_error::ProgramError, @@ -32,7 +33,8 @@ const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15; const TEST_EXECUTABLE_LAMPORTS: u8 = 16; -const ADD_LAMPORTS: u8 = 17; +const TEST_CALL_PRECOMPILE: u8 = 17; +const ADD_LAMPORTS: u8 = 18; // const MINT_INDEX: usize = 0; // unused placeholder const ARGUMENT_INDEX: usize = 1; @@ -45,6 +47,8 @@ const DERIVED_KEY2_INDEX: usize = 7; const DERIVED_KEY3_INDEX: usize = 8; const SYSTEM_PROGRAM_INDEX: usize = 9; const FROM_INDEX: usize = 10; +const ED25519_PROGRAM_INDEX: usize = 11; +// const INVOKE_PROGRAM_INDEX: usize = 12; unused placeholder fn do_nested_invokes(num_nested_invokes: u64, accounts: &[AccountInfo]) -> ProgramResult { assert!(accounts[ARGUMENT_INDEX].is_signer); @@ -663,6 +667,12 @@ fn process_instruction( // reset executable account **(*accounts[ARGUMENT_INDEX].lamports).borrow_mut() += 1; } + TEST_CALL_PRECOMPILE => { + msg!("Test calling precompiled program from cpi"); + let instruction = + Instruction::new_with_bytes(*accounts[ED25519_PROGRAM_INDEX].key, &[], vec![]); + invoke(&instruction, accounts)?; + } ADD_LAMPORTS => { // make sure the total balance is fine **accounts[0].lamports.borrow_mut() += 1; diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 2be7c2114044cf..53b55bf07c5a1b 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -481,7 +481,7 @@ fn test_program_bpf_sanity() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); // Call user program @@ -526,7 +526,7 @@ fn test_program_bpf_loader_deprecated() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( @@ -566,7 +566,7 @@ fn test_program_bpf_duplicate_accounts() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let program_id = load_bpf_program(&bank_client, &bpf_loader::id(), &mint_keypair, program); @@ -666,7 +666,7 @@ fn test_program_bpf_error_handling() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program(&bank_client, &bpf_loader::id(), &mint_keypair, program); let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)]; @@ -768,7 +768,7 @@ fn test_return_data_and_log_data_syscall() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -817,7 +817,9 @@ fn test_program_bpf_invoke_sanity() { const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15; const TEST_EXECUTABLE_LAMPORTS: u8 = 16; - const TEST_RETURN_DATA_TOO_LARGE: u8 = 18; + const TEST_CALL_PRECOMPILE: u8 = 17; + // const ADD_LAMPORTS: u8 = 18; + const TEST_RETURN_DATA_TOO_LARGE: u8 = 19; #[allow(dead_code)] #[derive(Debug)] @@ -849,7 +851,7 @@ fn test_program_bpf_invoke_sanity() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -892,6 +894,7 @@ fn test_program_bpf_invoke_sanity() { AccountMeta::new_readonly(derived_key3, false), AccountMeta::new_readonly(system_program::id(), false), AccountMeta::new(from_keypair.pubkey(), true), + AccountMeta::new_readonly(solana_sdk::ed25519_program::id(), false), AccountMeta::new_readonly(invoke_program_id, false), ]; @@ -1094,6 +1097,12 @@ fn test_program_bpf_invoke_sanity() { &[invoke_program_id.clone()], ); + do_invoke_failure_test_local( + TEST_CALL_PRECOMPILE, + TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete), + &[], + ); + do_invoke_failure_test_local( TEST_RETURN_DATA_TOO_LARGE, TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete), @@ -1161,7 +1170,7 @@ fn test_program_bpf_program_id_spoofing() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1214,7 +1223,7 @@ fn test_program_bpf_caller_has_access_to_cpi_program() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1254,7 +1263,7 @@ fn test_program_bpf_ro_modify() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1311,7 +1320,7 @@ fn test_program_bpf_call_depth() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( &bank_client, @@ -1346,7 +1355,7 @@ fn test_program_bpf_compute_budget() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( &bank_client, @@ -1449,7 +1458,7 @@ fn test_program_bpf_instruction_introspection() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1506,7 +1515,7 @@ fn test_program_bpf_test_use_latest_executor() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let panic_id = load_bpf_program( &bank_client, @@ -1601,7 +1610,7 @@ fn test_program_bpf_test_use_latest_executor2() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_error = load_bpf_program( &bank_client, @@ -1731,7 +1740,7 @@ fn test_program_bpf_upgrade() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); // Deploy upgrade program @@ -1825,7 +1834,7 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1905,9 +1914,9 @@ fn test_program_bpf_invoke_upgradeable_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_return = load_bpf_program( &bank_client, @@ -2020,7 +2029,7 @@ fn test_program_bpf_disguised_as_bpf_loader() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( @@ -2052,7 +2061,7 @@ fn test_program_bpf_c_dup() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let account_address = Pubkey::new_unique(); let account = AccountSharedData::new_data(42, &[1_u8, 2, 3], &system_program::id()).unwrap(); @@ -2083,9 +2092,9 @@ fn test_program_bpf_upgrade_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_return = load_bpf_program( &bank_client, @@ -2197,9 +2206,9 @@ fn test_program_bpf_upgrade_self_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let noop_program_id = load_bpf_program( @@ -2287,9 +2296,9 @@ fn test_program_bpf_set_upgrade_authority_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank_client = BankClient::new(bank); // Deploy CPI invoker program @@ -2380,7 +2389,7 @@ fn test_program_upgradeable_locks() { } = create_genesis_config(2_000_000_000); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2515,7 +2524,7 @@ fn test_program_bpf_finalize() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2577,7 +2586,7 @@ fn test_program_bpf_ro_account_modify() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2645,7 +2654,7 @@ fn test_program_bpf_realloc() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2901,7 +2910,7 @@ fn test_program_bpf_realloc_invoke() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, id, entrypoint); + bank.add_builtin(&name, &id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 67ddacbf4ffacb..843cfd1ce38a4e 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1672,7 +1672,7 @@ mod tests { bank.feature_set = Arc::new(FeatureSet::all_enabled()); bank.add_builtin( "solana_bpf_loader_upgradeable_program", - bpf_loader_upgradeable::id(), + &bpf_loader_upgradeable::id(), process_instruction, ); let bank = Arc::new(bank); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index d97ce1f5c8196a..3cd423cecbf75d 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -22,8 +22,8 @@ use solana_sdk::{ allow_native_ids, blake3_syscall_enabled, check_seed_length, close_upgradeable_program_accounts, demote_program_write_locks, disable_fees_sysvar, do_support_realloc, libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, - return_data_syscall_enabled, secp256k1_recover_syscall_enabled, - sol_log_data_syscall_enabled, + prevent_calling_precompiles_as_programs, return_data_syscall_enabled, + secp256k1_recover_syscall_enabled, sol_log_data_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -31,6 +31,7 @@ use solana_sdk::{ keccak, message::Message, native_loader, + precompiles::is_precompile, process_instruction::{self, stable_log, ComputeMeter, InvokeContext, Logger}, program::MAX_RETURN_DATA, pubkey::{Pubkey, PubkeyError, MAX_SEEDS, MAX_SEED_LEN}, @@ -2157,16 +2158,21 @@ fn check_account_infos( fn check_authorized_program( program_id: &Pubkey, instruction_data: &[u8], - close_upgradeable_program_accounts: bool, + invoke_context: &dyn InvokeContext, ) -> Result<(), EbpfError> { + #[allow(clippy::blocks_in_if_conditions)] if native_loader::check_id(program_id) || bpf_loader::check_id(program_id) || bpf_loader_deprecated::check_id(program_id) || (bpf_loader_upgradeable::check_id(program_id) && !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data) || bpf_loader_upgradeable::is_set_authority_instruction(instruction_data) - || (close_upgradeable_program_accounts + || (invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()) && bpf_loader_upgradeable::is_close_instruction(instruction_data)))) + || (invoke_context.is_feature_active(&prevent_calling_precompiles_as_programs::id()) + && is_precompile(program_id, |feature_id: &Pubkey| { + invoke_context.is_feature_active(feature_id) + })) { return Err(SyscallError::ProgramNotSupported(*program_id).into()); } @@ -2204,11 +2210,7 @@ fn call<'a>( let (message, caller_write_privileges, program_indices) = InstructionProcessor::create_message(&instruction, &signers, &invoke_context) .map_err(SyscallError::InstructionError)?; - check_authorized_program( - &instruction.program_id, - &instruction.data, - invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()), - )?; + check_authorized_program(&instruction.program_id, &instruction.data, *invoke_context)?; let (account_indices, mut accounts) = syscall.translate_accounts( &message, account_infos_addr, diff --git a/programs/ed25519/Cargo.toml b/programs/ed25519/Cargo.toml deleted file mode 100644 index 18f484de57e299..00000000000000 --- a/programs/ed25519/Cargo.toml +++ /dev/null @@ -1,25 +0,0 @@ -[package] -name = "solana-ed25519-program" -description = "Solana Ed25519 program" -version = "1.8.0" -homepage = "https://solana.com/" -documentation = "https://docs.rs/solana-ed25519-program" -repository = "https://github.com/solana-labs/solana" -authors = ["Solana Maintainers "] -license = "Apache-2.0" -edition = "2018" - -[dependencies] -solana-sdk = { path = "../../sdk", version = "=1.8.0" } - -[dev-dependencies] -ed25519-dalek = "=1.0.1" -rand = "0.7.0" -solana-logger = { path = "../../logger", version = "=1.8.0" } - -[lib] -crate-type = ["lib"] -name = "solana_ed25519_program" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/ed25519/src/lib.rs b/programs/ed25519/src/lib.rs deleted file mode 100644 index da579fb1d19f8a..00000000000000 --- a/programs/ed25519/src/lib.rs +++ /dev/null @@ -1,55 +0,0 @@ -use solana_sdk::{ - instruction::InstructionError, process_instruction::InvokeContext, pubkey::Pubkey, -}; - -pub fn process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, -) -> Result<(), InstructionError> { - // Should be already checked by now. - Ok(()) -} - -#[cfg(test)] -pub mod test { - use rand::{thread_rng, Rng}; - use solana_sdk::{ - ed25519_instruction::new_ed25519_instruction, - feature_set::FeatureSet, - hash::Hash, - signature::{Keypair, Signer}, - transaction::Transaction, - }; - use std::sync::Arc; - - #[test] - fn test_ed25519() { - solana_logger::setup(); - - let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); - let message_arr = b"hello"; - let mut instruction = new_ed25519_instruction(&privkey, message_arr); - let mint_keypair = Keypair::new(); - let feature_set = Arc::new(FeatureSet::all_enabled()); - - let tx = Transaction::new_signed_with_payer( - &[instruction.clone()], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - - assert!(tx.verify_precompiles(&feature_set).is_ok()); - - let index = thread_rng().gen_range(0, instruction.data.len()); - instruction.data[index] = instruction.data[index].wrapping_add(12); - let tx = Transaction::new_signed_with_payer( - &[instruction], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - assert!(tx.verify_precompiles(&feature_set).is_err()); - } -} diff --git a/programs/failure/tests/failure.rs b/programs/failure/tests/failure.rs index ba43045492f29a..54bf7da7acaa65 100644 --- a/programs/failure/tests/failure.rs +++ b/programs/failure/tests/failure.rs @@ -12,7 +12,7 @@ fn test_program_native_failure() { let (genesis_config, alice_keypair) = create_genesis_config(50); let program_id = solana_sdk::pubkey::new_rand(); let bank = Bank::new_for_tests(&genesis_config); - bank.add_native_program("solana_failure_program", &program_id, false); + bank.add_builtin_account("solana_failure_program", &program_id, false); // Call user program let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); diff --git a/programs/ownable/src/ownable_processor.rs b/programs/ownable/src/ownable_processor.rs index c415f2d5d25978..7254cbb3e223d8 100644 --- a/programs/ownable/src/ownable_processor.rs +++ b/programs/ownable/src/ownable_processor.rs @@ -74,7 +74,7 @@ mod tests { fn create_bank(lamports: u64) -> (Bank, Keypair) { let (genesis_config, mint_keypair) = create_genesis_config(lamports); let mut bank = Bank::new_for_tests(&genesis_config); - bank.add_builtin("ownable_program", crate::id(), process_instruction); + bank.add_builtin("ownable_program", &crate::id(), process_instruction); (bank, mint_keypair) } diff --git a/programs/secp256k1/Cargo.toml b/programs/secp256k1/Cargo.toml deleted file mode 100644 index c8b91cf6b2a40c..00000000000000 --- a/programs/secp256k1/Cargo.toml +++ /dev/null @@ -1,26 +0,0 @@ -[package] -name = "solana-secp256k1-program" -description = "Solana Secp256k1 program" -version = "1.8.0" -homepage = "https://solana.com/" -documentation = "https://docs.rs/solana-secp256k1-program" -repository = "https://github.com/solana-labs/solana" -authors = ["Solana Maintainers "] -license = "Apache-2.0" -edition = "2018" - -[dependencies] -solana-sdk = { path = "../../sdk", version = "=1.8.0" } - -[dev-dependencies] -bincode = "1.3.3" -libsecp256k1 = "0.6.0" -rand = "0.7.0" -solana-logger = { path = "../../logger", version = "=1.8.0" } - -[lib] -crate-type = ["lib"] -name = "solana_secp256k1_program" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/secp256k1/src/lib.rs b/programs/secp256k1/src/lib.rs deleted file mode 100644 index de879e0924e5b7..00000000000000 --- a/programs/secp256k1/src/lib.rs +++ /dev/null @@ -1,69 +0,0 @@ -use solana_sdk::{ - instruction::InstructionError, process_instruction::InvokeContext, pubkey::Pubkey, -}; - -pub fn process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, -) -> Result<(), InstructionError> { - // Should be already checked by now. - Ok(()) -} - -#[cfg(test)] -pub mod test { - use rand::{thread_rng, Rng}; - use solana_sdk::{ - feature_set, - hash::Hash, - secp256k1_instruction::{ - new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, - }, - signature::{Keypair, Signer}, - transaction::Transaction, - }; - use std::sync::Arc; - - #[test] - fn test_secp256k1() { - solana_logger::setup(); - let offsets = SecpSignatureOffsets::default(); - assert_eq!( - bincode::serialized_size(&offsets).unwrap() as usize, - SIGNATURE_OFFSETS_SERIALIZED_SIZE - ); - - let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); - let message_arr = b"hello"; - let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); - let mint_keypair = Keypair::new(); - let mut feature_set = feature_set::FeatureSet::all_enabled(); - feature_set - .active - .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()); - feature_set - .inactive - .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id()); - let feature_set = Arc::new(feature_set); - - let tx = Transaction::new_signed_with_payer( - &[secp_instruction.clone()], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - - assert!(tx.verify_precompiles(&feature_set).is_ok()); - - let index = thread_rng().gen_range(0, secp_instruction.data.len()); - secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); - let tx = Transaction::new_signed_with_payer( - &[secp_instruction], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - assert!(tx.verify_precompiles(&feature_set).is_err()); - } -} diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 94f0009af96d19..893dd6a90862e5 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -34,7 +34,6 @@ serde = { version = "1.0.130", features = ["rc"] } serde_derive = "1.0.103" solana-config-program = { path = "../programs/config", version = "=1.8.0" } solana-compute-budget-program = { path = "../programs/compute-budget", version = "=1.8.0" } -solana-ed25519-program = { path = "../programs/ed25519", version = "=1.8.0" } solana-frozen-abi = { path = "../frozen-abi", version = "=1.8.0" } solana-frozen-abi-macro = { path = "../frozen-abi/macro", version = "=1.8.0" } solana-logger = { path = "../logger", version = "=1.8.0" } @@ -44,7 +43,6 @@ solana-bucket-map = { path = "../bucket_map", version = "=1.8.0" } solana-program-runtime = { path = "../program-runtime", version = "=1.8.0" } solana-rayon-threadlimit = { path = "../rayon-threadlimit", version = "=1.8.0" } solana-sdk = { path = "../sdk", version = "=1.8.0" } -solana-secp256k1-program = { path = "../programs/secp256k1", version = "=1.8.0" } solana-stake-program = { path = "../programs/stake", version = "=1.8.0" } solana-vote-program = { path = "../programs/vote", version = "=1.8.0" } symlink = "0.1.0" @@ -58,6 +56,8 @@ crate-type = ["lib"] name = "solana_runtime" [dev-dependencies] +ed25519-dalek = "=1.0.1" +libsecp256k1 = "0.6.0" assert_matches = "1.5.0" [package.metadata.docs.rs] diff --git a/runtime/benches/bank.rs b/runtime/benches/bank.rs index a061f83d9d5188..d47ea8f6ecb834 100644 --- a/runtime/benches/bank.rs +++ b/runtime/benches/bank.rs @@ -127,10 +127,10 @@ fn do_bench_transactions( let mut bank = Bank::new_for_benches(&genesis_config); bank.add_builtin( "builtin_program", - Pubkey::new(&BUILTIN_PROGRAM_ID), + &Pubkey::new(&BUILTIN_PROGRAM_ID), process_instruction, ); - bank.add_native_program("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID), false); + bank.add_builtin_account("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID), false); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let transactions = create_transactions(&bank_client, &mint_keypair); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 447535ed2b2a4e..53e252d415489f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -97,6 +97,7 @@ use solana_sdk::{ native_token::sol_to_lamports, nonce, nonce_account, packet::PACKET_DATA_SIZE, + precompiles::get_precompiles, process_instruction::{ComputeMeter, Executor, ProcessInstructionWithContext}, program_utils::limited_deserialize, pubkey::Pubkey, @@ -2500,38 +2501,38 @@ impl Bank { // Add additional native programs specified in the genesis config for (name, program_id) in &genesis_config.native_instruction_processors { - self.add_native_program(name, program_id, false); + self.add_builtin_account(name, program_id, false); } } + fn burn_and_purge_account(&self, program_id: &Pubkey, mut account: AccountSharedData) { + self.capitalization.fetch_sub(account.lamports(), Relaxed); + // Resetting account balance to 0 is needed to really purge from AccountsDb and + // flush the Stakes cache + account.set_lamports(0); + self.store_account(program_id, &account); + } + // NOTE: must hold idempotent for the same set of arguments - pub fn add_native_program(&self, name: &str, program_id: &Pubkey, must_replace: bool) { + /// Add a builtin program account + pub fn add_builtin_account(&self, name: &str, program_id: &Pubkey, must_replace: bool) { let existing_genuine_program = - if let Some(mut account) = self.get_account_with_fixed_root(program_id) { - // it's very unlikely to be squatted at program_id as non-system account because of burden to - // find victim's pubkey/hash. So, when account.owner is indeed native_loader's, it's - // safe to assume it's a genuine program. - if native_loader::check_id(account.owner()) { - Some(account) - } else { - // malicious account is pre-occupying at program_id - // forcibly burn and purge it - - self.capitalization.fetch_sub(account.lamports(), Relaxed); - - // Resetting account balance to 0 is needed to really purge from AccountsDb and - // flush the Stakes cache - account.set_lamports(0); - self.store_account(program_id, &account); - None - } - } else { - None - }; + self.get_account_with_fixed_root(program_id) + .and_then(|account| { + // it's very unlikely to be squatted at program_id as non-system account because of burden to + // find victim's pubkey/hash. So, when account.owner is indeed native_loader's, it's + // safe to assume it's a genuine program. + if native_loader::check_id(account.owner()) { + Some(account) + } else { + // malicious account is pre-occupying at program_id + self.burn_and_purge_account(program_id, account); + None + } + }); if must_replace { // updating native program - match &existing_genuine_program { None => panic!( "There is no account to replace with native program ({}, {}).", @@ -2539,29 +2540,16 @@ impl Bank { ), Some(account) => { if *name == String::from_utf8_lossy(account.data()) { - // nop; it seems that already AccountsDb is updated. + // The existing account is well formed return; } - // continue to replace account } } } else { // introducing native program - - match &existing_genuine_program { - None => (), // continue to add account - Some(_account) => { - // nop; it seems that we already have account - - // before returning here to retain idempotent just make sure - // the existing native program name is same with what we're - // supposed to add here (but skipping) But I can't: - // following assertion already catches several different names for same - // program_id - // depending on clusters... - // assert_eq!(name.to_owned(), String::from_utf8_lossy(&account.data)); - return; - } + if existing_genuine_program.is_some() { + // The existing account is sufficient + return; } } @@ -2579,8 +2567,37 @@ impl Bank { self.inherit_specially_retained_account_fields(&existing_genuine_program), ); self.store_account_and_update_capitalization(program_id, &account); + } - debug!("Added native program {} under {:?}", name, program_id); + /// Add a precompiled program account + pub fn add_precompiled_account(&self, program_id: &Pubkey) { + if let Some(account) = self.get_account_with_fixed_root(program_id) { + if account.executable() { + // The account is already executable, that's all we need + return; + } else { + // malicious account is pre-occupying at program_id + self.burn_and_purge_account(program_id, account); + } + }; + + assert!( + !self.freeze_started(), + "Can't change frozen bank by adding not-existing new precompiled program ({}). \ + Maybe, inconsistent program activation is detected on snapshot restore?", + program_id + ); + + // Add a bogus executable account, which will be loaded and ignored. + let (lamports, rent_epoch) = self.inherit_specially_retained_account_fields(&None); + let account = AccountSharedData::from(Account { + lamports, + owner: solana_sdk::system_program::id(), + data: vec![], + executable: true, + rent_epoch, + }); + self.store_account_and_update_capitalization(program_id, &account); } pub fn set_rent_burn_percentage(&mut self, burn_percent: u8) { @@ -4587,10 +4604,15 @@ impl Bank { for builtin in builtins.genesis_builtins { self.add_builtin( &builtin.name, - builtin.id, + &builtin.id, builtin.process_instruction_with_context, ); } + for precompile in get_precompiles() { + if precompile.feature.is_none() { + self.add_precompile(&precompile.program_id); + } + } } self.feature_builtins = Arc::new(builtins.feature_builtins); @@ -5320,26 +5342,43 @@ impl Bank { pub fn add_builtin( &mut self, name: &str, - program_id: Pubkey, + program_id: &Pubkey, process_instruction_with_context: ProcessInstructionWithContext, ) { debug!("Adding program {} under {:?}", name, program_id); - self.add_native_program(name, &program_id, false); + self.add_builtin_account(name, program_id, false); self.message_processor .add_program(program_id, process_instruction_with_context); + debug!("Added program {} under {:?}", name, program_id); } /// Replace a builtin instruction processor if it already exists pub fn replace_builtin( &mut self, name: &str, - program_id: Pubkey, + program_id: &Pubkey, process_instruction_with_context: ProcessInstructionWithContext, ) { debug!("Replacing program {} under {:?}", name, program_id); - self.add_native_program(name, &program_id, true); + self.add_builtin_account(name, program_id, true); self.message_processor .add_program(program_id, process_instruction_with_context); + debug!("Replaced program {} under {:?}", name, program_id); + } + + /// Remove a builtin instruction processor if it already exists + pub fn remove_builtin(&mut self, name: &str, program_id: &Pubkey) { + debug!("Removing program {} under {:?}", name, program_id); + // Don't remove the account since the bank expects the account state to + // be idempotent + self.message_processor.remove_program(program_id); + debug!("Removed program {} under {:?}", name, program_id); + } + + pub fn add_precompile(&mut self, program_id: &Pubkey) { + debug!("Adding precompiled program {}", program_id); + self.add_precompiled_account(program_id); + debug!("Added precompiled program {:?}", program_id); } pub fn clean_accounts( @@ -5610,17 +5649,28 @@ impl Bank { match activation_type { ActivationType::NewProgram => self.add_builtin( &builtin.name, - builtin.id, + &builtin.id, builtin.process_instruction_with_context, ), ActivationType::NewVersion => self.replace_builtin( &builtin.name, - builtin.id, + &builtin.id, builtin.process_instruction_with_context, ), + ActivationType::RemoveProgram => { + self.remove_builtin(&builtin.name, &builtin.id) + } } } } + for precompile in get_precompiles() { + #[allow(clippy::blocks_in_if_conditions)] + if precompile.feature.map_or(false, |ref feature_id| { + self.feature_set.is_active(feature_id) + }) { + self.add_precompile(&precompile.program_id); + } + } } fn apply_spl_token_v2_set_authority_fix(&mut self) { @@ -6342,7 +6392,7 @@ pub(crate) mod tests { ) as u64, ); bank.rent_collector.slots_per_year = 421_812.0; - bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); bank } @@ -9828,7 +9878,7 @@ pub(crate) mod tests { assert!(bank.get_account(&mock_vote_program_id()).is_none()); bank.add_builtin( "mock_vote_program", - mock_vote_program_id(), + &mock_vote_program_id(), mock_vote_processor, ); assert!(bank.get_account(&mock_vote_program_id()).is_some()); @@ -9901,7 +9951,7 @@ pub(crate) mod tests { let vote_loader_account = bank.get_account(&solana_vote_program::id()).unwrap(); bank.add_builtin( "solana_vote_program", - solana_vote_program::id(), + &solana_vote_program::id(), mock_vote_processor, ); let new_vote_loader_account = bank.get_account(&solana_vote_program::id()).unwrap(); @@ -9950,8 +10000,8 @@ pub(crate) mod tests { assert!(!bank.stakes.read().unwrap().stake_delegations().is_empty()); assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); - bank.add_builtin("mock_program1", vote_id, mock_ix_processor); - bank.add_builtin("mock_program2", stake_id, mock_ix_processor); + bank.add_builtin("mock_program1", &vote_id, mock_ix_processor); + bank.add_builtin("mock_program2", &stake_id, mock_ix_processor); { let stakes = bank.stakes.read().unwrap(); assert!(stakes.vote_accounts().as_ref().is_empty()); @@ -9970,8 +10020,8 @@ pub(crate) mod tests { // Re-adding builtin programs should be no-op bank.update_accounts_hash(); let old_hash = bank.get_accounts_hash(); - bank.add_builtin("mock_program1", vote_id, mock_ix_processor); - bank.add_builtin("mock_program2", stake_id, mock_ix_processor); + bank.add_builtin("mock_program1", &vote_id, mock_ix_processor); + bank.add_builtin("mock_program2", &stake_id, mock_ix_processor); bank.update_accounts_hash(); let new_hash = bank.get_accounts_hash(); assert_eq!(old_hash, new_hash); @@ -10785,7 +10835,7 @@ pub(crate) mod tests { } let mock_program_id = Pubkey::new(&[2u8; 32]); - bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); let from_pubkey = solana_sdk::pubkey::new_rand(); let to_pubkey = solana_sdk::pubkey::new_rand(); @@ -10829,7 +10879,7 @@ pub(crate) mod tests { } let mock_program_id = Pubkey::new(&[2u8; 32]); - bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); let from_pubkey = solana_sdk::pubkey::new_rand(); let to_pubkey = solana_sdk::pubkey::new_rand(); @@ -10884,7 +10934,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - solana_vote_program::id(), + &solana_vote_program::id(), mock_ok_vote_processor, ); let result = bank.process_transaction(&tx); @@ -10938,7 +10988,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - solana_vote_program::id(), + &solana_vote_program::id(), mock_ok_vote_processor, ); @@ -10972,7 +11022,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - solana_vote_program::id(), + &solana_vote_program::id(), mock_ok_vote_processor, ); @@ -11029,7 +11079,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - solana_vote_program::id(), + &solana_vote_program::id(), mock_ok_vote_processor, ); @@ -11064,7 +11114,7 @@ pub(crate) mod tests { .map(|i| { let key = solana_sdk::pubkey::new_rand(); let name = format!("program{:?}", i); - bank.add_builtin(&name, key, mock_ok_vote_processor); + bank.add_builtin(&name, &key, mock_ok_vote_processor); (key, name.as_bytes().to_vec()) }) .collect(); @@ -11273,7 +11323,7 @@ pub(crate) mod tests { // Add a new program let program1_pubkey = solana_sdk::pubkey::new_rand(); - bank.add_builtin("program", program1_pubkey, nested_processor); + bank.add_builtin("program", &program1_pubkey, nested_processor); // Add a new program owned by the first let program2_pubkey = solana_sdk::pubkey::new_rand(); @@ -11640,20 +11690,24 @@ pub(crate) mod tests { )); assert_eq!(bank.get_account_modified_slot(&program_id), None); - Arc::get_mut(&mut bank) - .unwrap() - .add_builtin("mock_program", program_id, mock_ix_processor); + Arc::get_mut(&mut bank).unwrap().add_builtin( + "mock_program", + &program_id, + mock_ix_processor, + ); assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); let mut bank = Arc::new(new_from_parent(&bank)); - Arc::get_mut(&mut bank) - .unwrap() - .add_builtin("mock_program", program_id, mock_ix_processor); + Arc::get_mut(&mut bank).unwrap().add_builtin( + "mock_program", + &program_id, + mock_ix_processor, + ); assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); Arc::get_mut(&mut bank).unwrap().replace_builtin( "mock_program v2", - program_id, + &program_id, mock_ix_processor, ); assert_eq!( @@ -11687,18 +11741,18 @@ pub(crate) mod tests { Arc::get_mut(&mut bank) .unwrap() - .add_builtin("mock_program", loader_id, mock_ix_processor); + .add_builtin("mock_program", &loader_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); let mut bank = Arc::new(new_from_parent(&bank)); Arc::get_mut(&mut bank) .unwrap() - .add_builtin("mock_program", loader_id, mock_ix_processor); + .add_builtin("mock_program", &loader_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); } #[test] - fn test_add_native_program() { + fn test_add_builtin_account() { let (mut genesis_config, _mint_keypair) = create_genesis_config(100_000); activate_all_features(&mut genesis_config); @@ -11714,7 +11768,7 @@ pub(crate) mod tests { assert_capitalization_diff( &bank, - || bank.add_native_program("mock_program", &program_id, false), + || bank.add_builtin_account("mock_program", &program_id, false), |old, new| { assert_eq!(old + 1, new); }, @@ -11725,7 +11779,7 @@ pub(crate) mod tests { let bank = Arc::new(new_from_parent(&bank)); assert_capitalization_diff( &bank, - || bank.add_native_program("mock_program", &program_id, false), + || bank.add_builtin_account("mock_program", &program_id, false), |old, new| assert_eq!(old, new), ); @@ -11736,7 +11790,7 @@ pub(crate) mod tests { // invocations. assert_capitalization_diff( &bank, - || bank.add_native_program("mock_program v2", &program_id, true), + || bank.add_builtin_account("mock_program v2", &program_id, true), |old, new| assert_eq!(old, new), ); @@ -11748,7 +11802,7 @@ pub(crate) mod tests { let bank = Arc::new(new_from_parent(&bank)); assert_capitalization_diff( &bank, - || bank.add_native_program("mock_program v2", &program_id, true), + || bank.add_builtin_account("mock_program v2", &program_id, true), |old, new| assert_eq!(old, new), ); @@ -11760,12 +11814,12 @@ pub(crate) mod tests { } #[test] - fn test_add_native_program_inherited_cap_while_replacing() { + fn test_add_builtin_account_inherited_cap_while_replacing() { let (genesis_config, mint_keypair) = create_genesis_config(100_000); let bank = Bank::new_for_tests(&genesis_config); let program_id = solana_sdk::pubkey::new_rand(); - bank.add_native_program("mock_program", &program_id, false); + bank.add_builtin_account("mock_program", &program_id, false); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); // someone mess with program_id's balance @@ -11774,12 +11828,12 @@ pub(crate) mod tests { bank.deposit(&program_id, 10).unwrap(); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.add_native_program("mock_program v2", &program_id, true); + bank.add_builtin_account("mock_program v2", &program_id, true); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } #[test] - fn test_add_native_program_squatted_while_not_replacing() { + fn test_add_builtin_account_squatted_while_not_replacing() { let (genesis_config, mint_keypair) = create_genesis_config(100_000); let bank = Bank::new_for_tests(&genesis_config); let program_id = solana_sdk::pubkey::new_rand(); @@ -11790,7 +11844,7 @@ pub(crate) mod tests { bank.deposit(&program_id, 10).unwrap(); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.add_native_program("mock_program", &program_id, false); + bank.add_builtin_account("mock_program", &program_id, false); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } @@ -11800,7 +11854,7 @@ pub(crate) mod tests { program (mock_program, CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ Maybe, inconsistent program activation is detected on snapshot restore?" )] - fn test_add_native_program_after_frozen() { + fn test_add_builtin_account_after_frozen() { use std::str::FromStr; let (genesis_config, _mint_keypair) = create_genesis_config(100_000); @@ -11814,7 +11868,7 @@ pub(crate) mod tests { ); bank.freeze(); - bank.add_native_program("mock_program", &program_id, false); + bank.add_builtin_account("mock_program", &program_id, false); } #[test] @@ -11822,7 +11876,99 @@ pub(crate) mod tests { expected = "There is no account to replace with native program (mock_program, \ CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre)." )] - fn test_add_native_program_replace_none() { + fn test_add_builtin_account_replace_none() { + use std::str::FromStr; + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + + let slot = 123; + let program_id = Pubkey::from_str("CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre").unwrap(); + + let bank = Bank::new_from_parent( + &Arc::new(Bank::new_for_tests(&genesis_config)), + &Pubkey::default(), + slot, + ); + + bank.add_builtin_account("mock_program", &program_id, true); + } + + #[test] + fn test_add_precompiled_account() { + let (mut genesis_config, _mint_keypair) = create_genesis_config(100_000); + activate_all_features(&mut genesis_config); + + let slot = 123; + let program_id = solana_sdk::pubkey::new_rand(); + + let bank = Arc::new(Bank::new_from_parent( + &Arc::new(Bank::new_for_tests(&genesis_config)), + &Pubkey::default(), + slot, + )); + assert_eq!(bank.get_account_modified_slot(&program_id), None); + + assert_capitalization_diff( + &bank, + || bank.add_precompiled_account(&program_id), + |old, new| { + assert_eq!(old + 1, new); + }, + ); + + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + + let bank = Arc::new(new_from_parent(&bank)); + assert_capitalization_diff( + &bank, + || bank.add_precompiled_account(&program_id), + |old, new| assert_eq!(old, new), + ); + + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + } + + #[test] + fn test_add_precompiled_account_inherited_cap_while_replacing() { + let (genesis_config, mint_keypair) = create_genesis_config(100_000); + let bank = Bank::new_for_tests(&genesis_config); + let program_id = solana_sdk::pubkey::new_rand(); + + bank.add_precompiled_account(&program_id); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); + + // someone mess with program_id's balance + bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); + assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); + bank.deposit(&program_id, 10).unwrap(); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); + + bank.add_precompiled_account(&program_id); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); + } + + #[test] + fn test_add_precompiled_account_squatted_while_not_replacing() { + let (genesis_config, mint_keypair) = create_genesis_config(100_000); + let bank = Bank::new_for_tests(&genesis_config); + let program_id = solana_sdk::pubkey::new_rand(); + + // someone managed to squat at program_id! + bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); + assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); + bank.deposit(&program_id, 10).unwrap(); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); + + bank.add_precompiled_account(&program_id); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); + } + + #[test] + #[should_panic( + expected = "Can't change frozen bank by adding not-existing new precompiled \ + program (CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ + Maybe, inconsistent program activation is detected on snapshot restore?" + )] + fn test_add_precompiled_account_after_frozen() { use std::str::FromStr; let (genesis_config, _mint_keypair) = create_genesis_config(100_000); @@ -11834,8 +11980,9 @@ pub(crate) mod tests { &Pubkey::default(), slot, ); + bank.freeze(); - bank.add_native_program("mock_program", &program_id, true); + bank.add_precompiled_account(&program_id); } #[test] @@ -14032,7 +14179,7 @@ pub(crate) mod tests { } let program_id = solana_sdk::pubkey::new_rand(); - bank.add_builtin("mock_program1", program_id, mock_ix_processor); + bank.add_builtin("mock_program1", &program_id, mock_ix_processor); let blockhash = bank.last_blockhash(); #[allow(deprecated)] @@ -14244,7 +14391,7 @@ pub(crate) mod tests { Ok(()) } let program_id = solana_sdk::pubkey::new_rand(); - bank.add_builtin("mock_program", program_id, mock_ix_processor); + bank.add_builtin("mock_program", &program_id, mock_ix_processor); let message = Message::new( &[ @@ -14397,4 +14544,47 @@ pub(crate) mod tests { ); } } + + #[test] + fn test_call_precomiled_program() { + let GenesisConfigInfo { + mut genesis_config, + mint_keypair, + .. + } = create_genesis_config_with_leader(42, &Pubkey::new_unique(), 42); + activate_all_features(&mut genesis_config); + let bank = Bank::new_for_tests(&genesis_config); + + // libsecp256k1 + let secp_privkey = libsecp256k1::SecretKey::random(&mut rand::thread_rng()); + let message_arr = b"hello"; + let instruction = solana_sdk::secp256k1_instruction::new_secp256k1_instruction( + &secp_privkey, + message_arr, + ); + let tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + // calling the program should be successful when called from the bank + // even if the program itself is not called + bank.process_transaction(&tx).unwrap(); + + // ed25519 + let privkey = ed25519_dalek::Keypair::generate(&mut rand::thread_rng()); + let message_arr = b"hello"; + let instruction = + solana_sdk::ed25519_instruction::new_ed25519_instruction(&privkey, message_arr); + let tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + // calling the program should be successful when called from the bank + // even if the program itself is not called + bank.process_transaction(&tx).unwrap(); + } } diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index e0e59a2a6965f6..eeb2093927bd2a 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -46,6 +46,7 @@ macro_rules! with_program_logging { pub enum ActivationType { NewProgram, NewVersion, + RemoveProgram, } #[derive(Clone)] @@ -91,7 +92,7 @@ pub struct Builtins { /// Builtin programs that are always available pub genesis_builtins: Vec, - /// Builtin programs activated dynamically by feature + /// Builtin programs activated or deactivated dynamically by feature pub feature_builtins: Vec<(Builtin, Pubkey, ActivationType)>, } @@ -121,11 +122,20 @@ fn genesis_builtins() -> Vec { Builtin::new( "secp256k1_program", solana_sdk::secp256k1_program::id(), - solana_secp256k1_program::process_instruction, + dummy_process_instruction, ), ] } +/// place holder for secp256k1, remove when the precompile program is deactivated via feature activation +fn dummy_process_instruction( + _program_id: &Pubkey, + _data: &[u8], + _invoke_context: &mut dyn InvokeContext, +) -> Result<(), InstructionError> { + Ok(()) +} + /// Builtin programs activated dynamically by feature /// /// Note: If the feature_builtin is intended to replace another builtin program, it must have a new @@ -145,14 +155,17 @@ fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> { feature_set::tx_wide_compute_cap::id(), ActivationType::NewProgram, ), + // TODO when feature `prevent_calling_precompiles_as_programs` is + // cleaned up also remove "secp256k1_program" from the main builtins + // list ( Builtin::new( - "ed25519_program", - solana_sdk::ed25519_program::id(), - solana_ed25519_program::process_instruction, + "secp256k1_program", + solana_sdk::secp256k1_program::id(), + dummy_process_instruction, ), - feature_set::ed25519_program_enabled::id(), - ActivationType::NewProgram, + feature_set::prevent_calling_precompiles_as_programs::id(), + ActivationType::RemoveProgram, ), ] } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index b793d3cab6d9da..ca97b372c7f266 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -11,7 +11,7 @@ use solana_sdk::{ compute_budget::ComputeBudget, feature_set::{ demote_program_write_locks, do_support_realloc, neon_evm_compute_budget, - tx_wide_compute_cap, FeatureSet, + prevent_calling_precompiles_as_programs, tx_wide_compute_cap, FeatureSet, }, fee_calculator::FeeCalculator, hash::Hash, @@ -19,6 +19,7 @@ use solana_sdk::{ instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::{create_keyed_accounts_unified, KeyedAccount}, message::Message, + precompiles::is_precompile, process_instruction::{ ComputeMeter, Executor, InvokeContext, InvokeContextStackFrame, Logger, ProcessInstructionWithContext, @@ -400,13 +401,18 @@ impl MessageProcessor { /// Add a static entrypoint to intercept instructions before the dynamic loader. pub fn add_program( &mut self, - program_id: Pubkey, + program_id: &Pubkey, process_instruction: ProcessInstructionWithContext, ) { self.instruction_processor .add_program(program_id, process_instruction); } + /// Remove a program. + pub fn remove_program(&mut self, program_id: &Pubkey) { + self.instruction_processor.remove_program(program_id); + } + /// Record the initial state of the accounts so that they can be compared /// after the instruction is processed pub fn create_pre_accounts( @@ -531,6 +537,14 @@ impl MessageProcessor { blockhash: &Hash, fee_calculator: &FeeCalculator, ) -> Result<(), InstructionError> { + let program_id = instruction.program_id(&message.account_keys); + if feature_set.is_active(&prevent_calling_precompiles_as_programs::id()) + && is_precompile(program_id, |id| feature_set.is_active(id)) + { + // Precompiled programs don't have an instruction processor + return Ok(()); + } + // Fixup the special instructions key if present // before the account pre-values are taken care of for (pubkey, accont) in accounts.iter().take(message.account_keys.len()) { @@ -672,6 +686,8 @@ mod tests { message::Message, native_loader::{self, create_loadable_account_for_test}, process_instruction::MockComputeMeter, + secp256k1_instruction::new_secp256k1_instruction, + secp256k1_program, }; #[test] @@ -864,7 +880,7 @@ mod tests { let mock_system_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); let mut message_processor = MessageProcessor::default(); - message_processor.add_program(mock_system_program_id, mock_system_process_instruction); + message_processor.add_program(&mock_system_program_id, mock_system_process_instruction); let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -1052,7 +1068,7 @@ mod tests { let mock_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); let mut message_processor = MessageProcessor::default(); - message_processor.add_program(mock_program_id, mock_system_process_instruction); + message_processor.add_program(&mock_program_id, mock_system_process_instruction); let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -1579,4 +1595,63 @@ mod tests { ); } } + + #[test] + fn test_precompile() { + let mut message_processor = MessageProcessor::default(); + let mock_program_id = Pubkey::new_unique(); + fn mock_process_instruction( + _program_id: &Pubkey, + _data: &[u8], + _invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + Err(InstructionError::Custom(0xbabb1e)) + } + message_processor.add_program(&mock_program_id, mock_process_instruction); + + let secp256k1_account = AccountSharedData::new_ref(1, 0, &native_loader::id()); + secp256k1_account.borrow_mut().set_executable(true); + let mock_program_account = AccountSharedData::new_ref(1, 0, &native_loader::id()); + mock_program_account.borrow_mut().set_executable(true); + let accounts = vec![ + (secp256k1_program::id(), secp256k1_account), + (mock_program_id, mock_program_account), + ]; + + let message = Message::new( + &[ + new_secp256k1_instruction( + &libsecp256k1::SecretKey::random(&mut rand::thread_rng()), + b"hello", + ), + Instruction::new_with_bytes(mock_program_id, &[], vec![]), + ], + None, + ); + + let result = message_processor.process_message( + &message, + &[vec![0], vec![1]], + &accounts, + &RentCollector::default(), + None, + Rc::new(RefCell::new(Executors::default())), + None, + Arc::new(FeatureSet::all_enabled()), + ComputeBudget::new(), + Rc::new(RefCell::new(MockComputeMeter::default())), + &mut ExecuteDetailsTimings::default(), + Arc::new(Accounts::default_for_tests()), + &Ancestors::default(), + Hash::default(), + FeeCalculator::default(), + ); + assert_eq!( + result, + Err(TransactionError::InstructionError( + 1, + InstructionError::Custom(0xbabb1e) + )) + ); + } } diff --git a/runtime/tests/noop.rs b/runtime/tests/noop.rs index 3e26389b82e144..3f550323fc3c00 100644 --- a/runtime/tests/noop.rs +++ b/runtime/tests/noop.rs @@ -10,7 +10,7 @@ fn test_program_native_noop() { let (genesis_config, alice_keypair) = create_genesis_config(50); let program_id = solana_sdk::pubkey::new_rand(); let bank = Bank::new_for_tests(&genesis_config); - bank.add_native_program("solana_noop_program", &program_id, false); + bank.add_builtin_account("solana_noop_program", &program_id, false); // Call user program let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); diff --git a/sdk/src/ed25519_instruction.rs b/sdk/src/ed25519_instruction.rs index 704b210cbb2adf..052d4617b99fc2 100644 --- a/sdk/src/ed25519_instruction.rs +++ b/sdk/src/ed25519_instruction.rs @@ -170,6 +170,15 @@ fn get_data_slice<'a>( #[cfg(test)] pub mod test { use super::*; + use crate::{ + ed25519_instruction::new_ed25519_instruction, + feature_set::FeatureSet, + hash::Hash, + signature::{Keypair, Signer}, + transaction::Transaction, + }; + use rand::{thread_rng, Rng}; + use std::sync::Arc; fn test_case( num_signatures: u16, @@ -322,4 +331,34 @@ pub mod test { Err(PrecompileError::InvalidDataOffsets) ); } + + #[test] + fn test_ed25519() { + solana_logger::setup(); + + let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); + let message_arr = b"hello"; + let mut instruction = new_ed25519_instruction(&privkey, message_arr); + let mint_keypair = Keypair::new(); + let feature_set = Arc::new(FeatureSet::all_enabled()); + + let tx = Transaction::new_signed_with_payer( + &[instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + + assert!(tx.verify_precompiles(&feature_set).is_ok()); + + let index = thread_rng().gen_range(0, instruction.data.len()); + instruction.data[index] = instruction.data[index].wrapping_add(12); + let tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + assert!(tx.verify_precompiles(&feature_set).is_err()); + } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 502d0b4728af6e..e65ff489d4a0f2 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -223,6 +223,12 @@ pub mod do_support_realloc { solana_sdk::declare_id!("75m6ysz33AfLA5DDEzWM1obBrnPQRSsdVQ2nRmc8Vuu1"); } +// Note: when this feature is cleaned up, also remove the secp256k1 program from +// the list of builtins and remove its files from /programs +pub mod prevent_calling_precompiles_as_programs { + solana_sdk::declare_id!("4ApgRX3ud6p7LNMJmsuaAcZY5HWctGPr5obAsjB3A54d"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -274,6 +280,7 @@ lazy_static! { (sol_log_data_syscall_enabled::id(), "enable sol_log_data syscall"), (stakes_remove_delegation_if_inactive::id(), "remove delegations from stakes cache when inactive"), (do_support_realloc::id(), "support account data reallocation"), + (prevent_calling_precompiles_as_programs::id(), "Prevent calling precompiles as programs"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/precompiles.rs b/sdk/src/precompiles.rs index 3372fca3c4737a..f1fff29213ff28 100644 --- a/sdk/src/precompiles.rs +++ b/sdk/src/precompiles.rs @@ -5,7 +5,9 @@ use { crate::{ decode_error::DecodeError, - feature_set::{ed25519_program_enabled, FeatureSet}, + feature_set::{ + ed25519_program_enabled, prevent_calling_precompiles_as_programs, FeatureSet, + }, instruction::CompiledInstruction, pubkey::Pubkey, }, @@ -38,7 +40,7 @@ impl DecodeError for PrecompileError { pub type Verify = fn(&[u8], &[&[u8]], &Arc) -> std::result::Result<(), PrecompileError>; /// Information on a precompiled program -struct Precompile { +pub struct Precompile { /// Program id pub program_id: Pubkey, /// Feature to enable on, `None` indicates always enabled @@ -56,8 +58,13 @@ impl Precompile { } } /// Check if a program id is this precompiled program - pub fn check_id(&self, program_id: &Pubkey, feature_set: &Arc) -> bool { - self.feature.map_or(true, |f| feature_set.is_active(&f)) && self.program_id == *program_id + pub fn check_id(&self, program_id: &Pubkey, is_enabled: F) -> bool + where + F: Fn(&Pubkey) -> bool, + { + self.feature + .map_or(true, |ref feature_id| is_enabled(feature_id)) + && self.program_id == *program_id } /// Verify this precompiled program pub fn verify( @@ -75,7 +82,7 @@ lazy_static! { static ref PRECOMPILES: Vec = vec![ Precompile::new( crate::secp256k1_program::id(), - None, + Some(prevent_calling_precompiles_as_programs::id()), crate::secp256k1_instruction::verify, ), Precompile::new( @@ -87,10 +94,17 @@ lazy_static! { } /// Check if a program is a precompiled program -pub fn is_precompile(program_id: &Pubkey, feature_set: &Arc) -> bool { +pub fn is_precompile(program_id: &Pubkey, is_enabled: F) -> bool +where + F: Fn(&Pubkey) -> bool, +{ PRECOMPILES .iter() - .any(|precompile| precompile.check_id(program_id, feature_set)) + .any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id))) +} + +pub fn get_precompiles<'a>() -> &'a [Precompile] { + &PRECOMPILES } /// Check that a program is precompiled and if so verify it @@ -101,7 +115,7 @@ pub fn verify_if_precompile( feature_set: &Arc, ) -> Result<(), PrecompileError> { for precompile in PRECOMPILES.iter() { - if precompile.check_id(program_id, feature_set) { + if precompile.check_id(program_id, |feature_id| feature_set.is_active(feature_id)) { let instruction_datas: Vec<_> = all_instructions .iter() .map(|instruction| instruction.data.as_ref()) diff --git a/sdk/src/secp256k1_instruction.rs b/sdk/src/secp256k1_instruction.rs index 299237b9cbb5d6..1cf471db9b681a 100644 --- a/sdk/src/secp256k1_instruction.rs +++ b/sdk/src/secp256k1_instruction.rs @@ -212,6 +212,17 @@ fn get_data_slice<'a>( #[cfg(test)] pub mod test { use super::*; + use crate::{ + feature_set, + hash::Hash, + secp256k1_instruction::{ + new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, + }, + signature::{Keypair, Signer}, + transaction::Transaction, + }; + use rand::{thread_rng, Rng}; + use std::sync::Arc; fn test_case( num_signatures: u8, @@ -390,4 +401,46 @@ pub mod test { Err(PrecompileError::InvalidInstructionDataSize) ); } + + #[test] + fn test_secp256k1() { + solana_logger::setup(); + let offsets = SecpSignatureOffsets::default(); + assert_eq!( + bincode::serialized_size(&offsets).unwrap() as usize, + SIGNATURE_OFFSETS_SERIALIZED_SIZE + ); + + let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); + let message_arr = b"hello"; + let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); + let mint_keypair = Keypair::new(); + let mut feature_set = feature_set::FeatureSet::all_enabled(); + feature_set + .active + .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + feature_set + .inactive + .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + let feature_set = Arc::new(feature_set); + + let tx = Transaction::new_signed_with_payer( + &[secp_instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + + assert!(tx.verify_precompiles(&feature_set).is_ok()); + + let index = thread_rng().gen_range(0, secp_instruction.data.len()); + secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); + let tx = Transaction::new_signed_with_payer( + &[secp_instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + assert!(tx.verify_precompiles(&feature_set).is_err()); + } }