From 529fefc7ccc842fc4ecb8160a8caca70b41f1105 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 7 Sep 2021 14:44:52 -0700 Subject: [PATCH] Remove native id check in pda creation (#19595) --- programs/bpf/rust/invoke/src/lib.rs | 23 ++---- programs/bpf_loader/src/syscalls.rs | 104 +++++++++++++++++++--------- sdk/program/src/pubkey.rs | 39 +---------- sdk/src/feature_set.rs | 7 +- 4 files changed, 84 insertions(+), 89 deletions(-) diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index a11b2fe6ed..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::IllegalOwner - ); - } -} diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index ba1c1aeda7..fb64b7b64d 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -21,9 +21,9 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - blake3_syscall_enabled, close_upgradeable_program_accounts, demote_program_write_locks, - disable_fees_sysvar, enforce_aligned_host_addrs, libsecp256k1_0_5_upgrade_enabled, - mem_overlap_fix, secp256k1_recover_syscall_enabled, + allow_native_ids, blake3_syscall_enabled, close_upgradeable_program_accounts, + demote_program_write_locks, disable_fees_sysvar, enforce_aligned_host_addrs, + libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, secp256k1_recover_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -32,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 +247,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: 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: compute_budget.create_program_address_units, compute_meter: invoke_context.get_compute_meter(), loader_id, enforce_aligned_host_addrs, + allow_native_ids, }), None, )?; @@ -834,12 +836,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( @@ -865,6 +897,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(_) => { @@ -893,6 +931,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( @@ -924,32 +963,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 f2b1748460..3c04261140 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -199,10 +199,14 @@ pub mod ed25519_program_enabled { solana_sdk::declare_id!("E1TvTNipX8TKNHrhRC8SMuAwQmGY58TZ4drdztP3Gxwc"); } +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 = [ - (deprecate_rewards_sysvar::id(), "deprecate unused rewards sysvar"), + (deprecate_rewards_sysvar::id(), "deprecate unused rewards sysvar"), (pico_inflation::id(), "pico inflation"), (full_inflation::devnet_and_testnet::id(), "full inflation on devnet and testnet"), (spl_token_v2_multisig_fix::id(), "spl-token multisig fix"), @@ -244,6 +248,7 @@ lazy_static! { (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"), (ed25519_program_enabled::id(), "enable builtin ed25519 signature verify program"), + (allow_native_ids::id(), "allow native program ids in program derived addresses"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()