From e9c3f11d24ae2cf76c29fecdb596019b99459a65 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 7 Sep 2021 23:26:08 -0700 Subject: [PATCH] Remove native id check in pda creation (backport #19595) (#19689) * Remove native id check in pda creation (#19595) (cherry picked from commit 529fefc7ccc842fc4ecb8160a8caca70b41f1105) # Conflicts: # programs/bpf/rust/invoke/src/lib.rs # programs/bpf_loader/src/syscalls.rs # sdk/src/feature_set.rs * resolve conflicts Co-authored-by: Jack May --- programs/bpf/rust/invoke/src/lib.rs | 23 ++---- programs/bpf_loader/src/syscalls.rs | 107 +++++++++++++++++++--------- sdk/program/src/pubkey.rs | 39 +--------- sdk/src/feature_set.rs | 5 ++ 4 files changed, 85 insertions(+), 89 deletions(-) diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index dec99fd88e..0089941819 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -263,10 +263,9 @@ fn process_instruction( )?, accounts[DERIVED_KEY1_INDEX].key ); - let not_native_program_id = Pubkey::new_from_array([6u8; 32]); - assert!(!not_native_program_id.is_native_program_id()); + let new_program_id = Pubkey::new_from_array([6u8; 32]); assert_eq!( - Pubkey::create_program_address(&[b"You pass butter"], ¬_native_program_id) + Pubkey::create_program_address(&[b"You pass butter"], &new_program_id) .unwrap_err(), PubkeyError::InvalidSeeds ); @@ -278,10 +277,9 @@ fn process_instruction( Pubkey::try_find_program_address(&[b"You pass butter"], program_id).unwrap(); assert_eq!(&address, accounts[DERIVED_KEY1_INDEX].key); assert_eq!(bump_seed, bump_seed1); - let not_native_program_id = Pubkey::new_from_array([6u8; 32]); - assert!(!not_native_program_id.is_native_program_id()); + let new_program_id = Pubkey::new_from_array([6u8; 32]); assert_eq!( - Pubkey::create_program_address(&[b"You pass butter"], ¬_native_program_id) + Pubkey::create_program_address(&[b"You pass butter"], &new_program_id) .unwrap_err(), PubkeyError::InvalidSeeds ); @@ -653,16 +651,3 @@ fn process_instruction( Ok(()) } - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn create_program_address_is_defined() { - assert_eq!( - Pubkey::create_program_address(&[b"You pass butter"], &Pubkey::default()).unwrap_err(), - PubkeyError::InvalidSeeds - ); - } -} diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index ed238654de..175b131df1 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -19,10 +19,11 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - close_upgradeable_program_accounts, cpi_data_cost, demote_program_write_locks, - enforce_aligned_host_addrs, keccak256_syscall_enabled, libsecp256k1_0_5_upgrade_enabled, - mem_overlap_fix, memory_ops_syscalls, secp256k1_recover_syscall_enabled, - set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc, + allow_native_ids, close_upgradeable_program_accounts, cpi_data_cost, + demote_program_write_locks, enforce_aligned_host_addrs, keccak256_syscall_enabled, + libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, memory_ops_syscalls, + secp256k1_recover_syscall_enabled, set_upgrade_authority_via_cpi_enabled, + sysvar_via_syscall, update_data_on_realloc, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -31,7 +32,7 @@ use solana_sdk::{ keyed_account::KeyedAccount, native_loader, process_instruction::{self, stable_log, ComputeMeter, InvokeContext, Logger}, - pubkey::{Pubkey, PubkeyError, MAX_SEEDS}, + pubkey::{Pubkey, PubkeyError, MAX_SEEDS, MAX_SEED_LEN}, rent::Rent, secp256k1_recover::{ Secp256k1RecoverError, SECP256K1_PUBLIC_KEY_LENGTH, SECP256K1_SIGNATURE_LENGTH, @@ -247,22 +248,24 @@ pub fn bind_syscall_context_objects<'a>( None, )?; + let allow_native_ids = invoke_context.is_feature_active(&allow_native_ids::id()); vm.bind_syscall_context_object( Box::new(SyscallCreateProgramAddress { cost: bpf_compute_budget.create_program_address_units, compute_meter: invoke_context.get_compute_meter(), loader_id, enforce_aligned_host_addrs, + allow_native_ids, }), None, )?; - vm.bind_syscall_context_object( Box::new(SyscallTryFindProgramAddress { cost: bpf_compute_budget.create_program_address_units, compute_meter: invoke_context.get_compute_meter(), loader_id, enforce_aligned_host_addrs, + allow_native_ids, }), None, )?; @@ -830,12 +833,42 @@ fn translate_program_address_inputs<'a>( Ok((seeds, program_id)) } +fn is_native_id(seeds: &[&[u8]], program_id: &Pubkey) -> bool { + use solana_sdk::{config, feature, secp256k1_program, stake, system_program, vote}; + // Does more than just check native ids in order to emulate the same failure + // signature that `compute_program_address` had before the removal of the + // check. + if seeds.len() > MAX_SEEDS { + return true; + } + for seed in seeds.iter() { + if seed.len() > MAX_SEED_LEN { + return true; + } + } + + let native_ids = [ + bpf_loader::id(), + bpf_loader_deprecated::id(), + feature::id(), + config::program::id(), + stake::program::id(), + stake::config::id(), + vote::program::id(), + secp256k1_program::id(), + system_program::id(), + sysvar::id(), + ]; + native_ids.contains(program_id) +} + /// Create a program address struct SyscallCreateProgramAddress<'a> { cost: u64, compute_meter: Rc>, loader_id: &'a Pubkey, enforce_aligned_host_addrs: bool, + allow_native_ids: bool, } impl<'a> SyscallObject for SyscallCreateProgramAddress<'a> { fn call( @@ -861,6 +894,12 @@ impl<'a> SyscallObject for SyscallCreateProgramAddress<'a> { ); question_mark!(self.compute_meter.consume(self.cost), result); + + if !self.allow_native_ids && is_native_id(&seeds, program_id) { + *result = Ok(1); + return; + } + let new_address = match Pubkey::create_program_address(&seeds, program_id) { Ok(address) => address, Err(_) => { @@ -889,6 +928,7 @@ struct SyscallTryFindProgramAddress<'a> { compute_meter: Rc>, loader_id: &'a Pubkey, enforce_aligned_host_addrs: bool, + allow_native_ids: bool, } impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { fn call( @@ -920,32 +960,35 @@ impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { seeds_with_bump.push(&bump_seed); question_mark!(self.compute_meter.consume(self.cost), result); - if let Ok(new_address) = - Pubkey::create_program_address(&seeds_with_bump, program_id) - { - let bump_seed_ref = question_mark!( - translate_type_mut::( - memory_mapping, - bump_seed_addr, - self.loader_id, - self.enforce_aligned_host_addrs, - ), - result - ); - let address = question_mark!( - translate_slice_mut::( - memory_mapping, - address_addr, - 32, - self.loader_id, - self.enforce_aligned_host_addrs, - ), - result - ); - *bump_seed_ref = bump_seed[0]; - address.copy_from_slice(new_address.as_ref()); - *result = Ok(0); - return; + + if self.allow_native_ids || !is_native_id(&seeds, program_id) { + if let Ok(new_address) = + Pubkey::create_program_address(&seeds_with_bump, program_id) + { + let bump_seed_ref = question_mark!( + translate_type_mut::( + memory_mapping, + bump_seed_addr, + self.loader_id, + self.enforce_aligned_host_addrs, + ), + result + ); + let address = question_mark!( + translate_slice_mut::( + memory_mapping, + address_addr, + 32, + self.loader_id, + self.enforce_aligned_host_addrs, + ), + result + ); + *bump_seed_ref = bump_seed[0]; + address.copy_from_slice(new_address.as_ref()); + *result = Ok(0); + return; + } } } bump_seed[0] -= 1; diff --git a/sdk/program/src/pubkey.rs b/sdk/program/src/pubkey.rs index e7d3f9d58f..bd2880ed95 100644 --- a/sdk/program/src/pubkey.rs +++ b/sdk/program/src/pubkey.rs @@ -1,8 +1,5 @@ #![allow(clippy::integer_arithmetic)] -use crate::{ - bpf_loader, bpf_loader_deprecated, config, decode_error::DecodeError, feature, hash::hashv, - secp256k1_program, stake, system_program, sysvar, vote, -}; +use crate::{decode_error::DecodeError, hash::hashv}; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use num_derive::{FromPrimitive, ToPrimitive}; @@ -214,10 +211,6 @@ impl Pubkey { } } - if program_id.is_native_program_id() { - return Err(PubkeyError::IllegalOwner); - } - // Perform the calculation inline, calling this from within a program is // not supported #[cfg(not(target_arch = "bpf"))] @@ -368,22 +361,6 @@ impl Pubkey { #[cfg(not(target_arch = "bpf"))] crate::program_stubs::sol_log(&self.to_string()); } - - pub fn is_native_program_id(&self) -> bool { - let all_program_ids = [ - bpf_loader::id(), - bpf_loader_deprecated::id(), - feature::id(), - config::program::id(), - stake::program::id(), - stake::config::id(), - vote::program::id(), - secp256k1_program::id(), - system_program::id(), - sysvar::id(), - ]; - all_program_ids.contains(self) - } } impl AsRef<[u8]> for Pubkey { @@ -600,20 +577,6 @@ mod tests { } } - #[test] - fn test_is_native_program_id() { - assert!(bpf_loader::id().is_native_program_id()); - assert!(bpf_loader_deprecated::id().is_native_program_id()); - assert!(config::program::id().is_native_program_id()); - assert!(feature::id().is_native_program_id()); - assert!(secp256k1_program::id().is_native_program_id()); - assert!(stake::program::id().is_native_program_id()); - assert!(stake::config::id().is_native_program_id()); - assert!(system_program::id().is_native_program_id()); - assert!(sysvar::id().is_native_program_id()); - assert!(vote::program::id().is_native_program_id()); - } - fn pubkey_from_seed_by_marker(marker: &[u8]) -> Result { let key = Pubkey::new_unique(); let owner = Pubkey::default(); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 98ed71a7e8..49d8f21725 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -191,6 +191,10 @@ pub mod demote_program_write_locks { solana_sdk::declare_id!("3E3jV7v9VcdJL8iYZUMax9DiDno8j7EWUVbhm9RtShj2"); } +pub mod allow_native_ids { + solana_sdk::declare_id!("GVnDbNkECwrzLM7aVBGWpBYo3yH1ACaXB4ottNX8pedZ"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -239,6 +243,7 @@ lazy_static! { (close_upgradeable_program_accounts::id(), "enable closing upgradeable program accounts"), (stake_program_advance_activating_credits_observed::id(), "Enable advancing credits observed for activation epoch #19309"), (demote_program_write_locks::id(), "demote program write locks to readonly #19593"), + (allow_native_ids::id(), "allow native program ids in program derived addresses"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()