From ed4e7c0c8728983fcc347abd2f05adcb4369497b Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 8 Sep 2021 08:26:07 +0000 Subject: [PATCH] Check seed length before trying to cal program address (backport #19699) (#19708) * Check seed length before trying to cal program address (#19699) (cherry picked from commit 778b2adbeaa35703ee2e958720bcff60e18db292) * resolve conflicts Co-authored-by: Jack May --- Cargo.lock | 1 + programs/bpf_loader/Cargo.toml | 1 + programs/bpf_loader/src/syscalls.rs | 321 ++++++++++++++++++++++++++-- sdk/program/src/pubkey.rs | 42 ++++ sdk/src/feature_set.rs | 5 + 5 files changed, 357 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 742a56aa7c..cc8ed8b30a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4182,6 +4182,7 @@ version = "1.7.12" dependencies = [ "bincode", "byteorder", + "curve25519-dalek 3.0.0", "libsecp256k1", "log 0.4.11", "num-derive", diff --git a/programs/bpf_loader/Cargo.toml b/programs/bpf_loader/Cargo.toml index cf969216b6..8964a79480 100644 --- a/programs/bpf_loader/Cargo.toml +++ b/programs/bpf_loader/Cargo.toml @@ -25,6 +25,7 @@ solana_rbpf = "=0.2.11" thiserror = "1.0" [dev-dependencies] +curve25519-dalek = "3.0.0" rand = "0.7.3" rustversion = "1.0.4" diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 175b131df1..f50bfa6c89 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -19,7 +19,7 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - allow_native_ids, close_upgradeable_program_accounts, cpi_data_cost, + allow_native_ids, check_seed_length, 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, @@ -249,6 +249,7 @@ pub fn bind_syscall_context_objects<'a>( )?; let allow_native_ids = invoke_context.is_feature_active(&allow_native_ids::id()); + let check_seed_length = invoke_context.is_feature_active(&check_seed_length::id()); vm.bind_syscall_context_object( Box::new(SyscallCreateProgramAddress { cost: bpf_compute_budget.create_program_address_units, @@ -256,6 +257,7 @@ pub fn bind_syscall_context_objects<'a>( loader_id, enforce_aligned_host_addrs, allow_native_ids, + check_seed_length, }), None, )?; @@ -266,6 +268,7 @@ pub fn bind_syscall_context_objects<'a>( loader_id, enforce_aligned_host_addrs, allow_native_ids, + check_seed_length, }), None, )?; @@ -794,13 +797,14 @@ impl SyscallObject for SyscallAllocFree { } } -fn translate_program_address_inputs<'a>( +fn translate_and_check_program_address_inputs<'a>( seeds_addr: u64, seeds_len: u64, program_id_addr: u64, memory_mapping: &MemoryMapping, loader_id: &Pubkey, enforce_aligned_host_addrs: bool, + check_seed_length: bool, ) -> Result<(Vec<&'a [u8]>, &'a Pubkey), EbpfError> { let untranslated_seeds = translate_slice::<&[&u8]>( memory_mapping, @@ -815,6 +819,9 @@ fn translate_program_address_inputs<'a>( let seeds = untranslated_seeds .iter() .map(|untranslated_seed| { + if check_seed_length && untranslated_seed.len() > MAX_SEED_LEN { + return Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()); + } translate_slice::( memory_mapping, untranslated_seed.as_ptr() as *const _ as u64, @@ -869,6 +876,7 @@ struct SyscallCreateProgramAddress<'a> { loader_id: &'a Pubkey, enforce_aligned_host_addrs: bool, allow_native_ids: bool, + check_seed_length: bool, } impl<'a> SyscallObject for SyscallCreateProgramAddress<'a> { fn call( @@ -881,19 +889,26 @@ impl<'a> SyscallObject for SyscallCreateProgramAddress<'a> { memory_mapping: &MemoryMapping, result: &mut Result>, ) { + if self.check_seed_length { + question_mark!(self.compute_meter.consume(self.cost), result); + } + let (seeds, program_id) = question_mark!( - translate_program_address_inputs( + translate_and_check_program_address_inputs( seeds_addr, seeds_len, program_id_addr, memory_mapping, self.loader_id, self.enforce_aligned_host_addrs, + self.check_seed_length, ), result ); - question_mark!(self.compute_meter.consume(self.cost), result); + if !self.check_seed_length { + question_mark!(self.compute_meter.consume(self.cost), result); + } if !self.allow_native_ids && is_native_id(&seeds, program_id) { *result = Ok(1); @@ -929,6 +944,7 @@ struct SyscallTryFindProgramAddress<'a> { loader_id: &'a Pubkey, enforce_aligned_host_addrs: bool, allow_native_ids: bool, + check_seed_length: bool, } impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { fn call( @@ -941,14 +957,19 @@ impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { memory_mapping: &MemoryMapping, result: &mut Result>, ) { + if self.check_seed_length { + question_mark!(self.compute_meter.consume(self.cost), result); + } + let (seeds, program_id) = question_mark!( - translate_program_address_inputs( + translate_and_check_program_address_inputs( seeds_addr, seeds_len, program_id_addr, memory_mapping, self.loader_id, self.enforce_aligned_host_addrs, + self.check_seed_length, ), result ); @@ -959,7 +980,9 @@ impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { let mut seeds_with_bump = seeds.to_vec(); seeds_with_bump.push(&bump_seed); - question_mark!(self.compute_meter.consume(self.cost), result); + if !self.check_seed_length { + question_mark!(self.compute_meter.consume(self.cost), result); + } if self.allow_native_ids || !is_native_id(&seeds, program_id) { if let Ok(new_address) = @@ -992,6 +1015,9 @@ impl<'a> SyscallObject for SyscallTryFindProgramAddress<'a> { } } bump_seed[0] -= 1; + if self.check_seed_length { + question_mark!(self.compute_meter.consume(self.cost), result); + } } *result = Ok(1); } @@ -2500,6 +2526,12 @@ mod tests { }; } + #[allow(dead_code)] + struct MockSlice { + pub vm_addr: u64, + pub len: usize, + } + #[test] fn test_translate() { const START: u64 = 100; @@ -3155,17 +3187,12 @@ mod tests { let bytes1 = "Gaggablaghblagh!"; let bytes2 = "flurbos"; - #[allow(dead_code)] - struct MockSlice { - pub addr: u64, - pub len: usize, - } let mock_slice1 = MockSlice { - addr: 4096, + vm_addr: 4096, len: bytes1.len(), }; let mock_slice2 = MockSlice { - addr: 8192, + vm_addr: 8192, len: bytes2.len(), }; let bytes_to_hash = [mock_slice1, mock_slice2]; @@ -3453,4 +3480,272 @@ mod tests { assert!(check_overlapping(10, 12, 3)); assert!(!check_overlapping(10, 13, 3)); } + + fn call_program_address_common( + seeds: &[&[u8]], + program_id: &Pubkey, + syscall: &mut dyn SyscallObject, + ) -> Result<(Pubkey, u8), EbpfError> { + const SEEDS_VA: u64 = 0x100000000; + const PROGRAM_ID_VA: u64 = 0x200000000; + const ADDRESS_VA: u64 = 0x300000000; + const BUMP_SEED_VA: u64 = 0x400000000; + const SEED_VA: u64 = 0x500000000; + + let config = Config::default(); + let address = Pubkey::default(); + let bump_seed = 0; + let mut mock_slices = Vec::with_capacity(seeds.len()); + let mut regions = vec![ + MemoryRegion::default(), + MemoryRegion { + host_addr: mock_slices.as_ptr() as u64, + vm_addr: SEEDS_VA, + len: (seeds.len() * size_of::()) as u64, + vm_gap_shift: 63, + is_writable: false, + }, + MemoryRegion { + host_addr: program_id.as_ref().as_ptr() as u64, + vm_addr: PROGRAM_ID_VA, + len: 32, + vm_gap_shift: 63, + is_writable: false, + }, + MemoryRegion { + host_addr: address.as_ref().as_ptr() as u64, + vm_addr: ADDRESS_VA, + len: 32, + vm_gap_shift: 63, + is_writable: true, + }, + MemoryRegion { + host_addr: &bump_seed as *const u8 as u64, + vm_addr: BUMP_SEED_VA, + len: 32, + vm_gap_shift: 63, + is_writable: true, + }, + ]; + + for (i, seed) in seeds.iter().enumerate() { + let vm_addr = SEED_VA + (i as u64 * 0x100000000); + let mock_slice = MockSlice { + vm_addr, + len: seed.len(), + }; + mock_slices.push(mock_slice); + regions.push(MemoryRegion { + host_addr: seed.as_ptr() as u64, + vm_addr, + len: seed.len() as u64, + vm_gap_shift: 63, + is_writable: false, + }); + } + let memory_mapping = MemoryMapping::new::(regions, &config).unwrap(); + + let mut result = Ok(0); + syscall.call( + SEEDS_VA, + seeds.len() as u64, + PROGRAM_ID_VA, + ADDRESS_VA, + BUMP_SEED_VA, + &memory_mapping, + &mut result, + ); + let _ = result?; + Ok((address, bump_seed)) + } + + fn create_program_address( + seeds: &[&[u8]], + program_id: &Pubkey, + remaining: u64, + ) -> Result> { + let compute_meter: Rc> = + Rc::new(RefCell::new(MockComputeMeter { remaining })); + let mut syscall = SyscallCreateProgramAddress { + cost: 1, + compute_meter: compute_meter.clone(), + loader_id: &bpf_loader::id(), + enforce_aligned_host_addrs: true, + allow_native_ids: true, + check_seed_length: true, + }; + let (address, _) = call_program_address_common(seeds, program_id, &mut syscall)?; + Ok(address) + } + + fn try_find_program_address( + seeds: &[&[u8]], + program_id: &Pubkey, + remaining: u64, + ) -> Result<(Pubkey, u8), EbpfError> { + let compute_meter: Rc> = + Rc::new(RefCell::new(MockComputeMeter { remaining })); + let mut syscall = SyscallTryFindProgramAddress { + cost: 1, + compute_meter: compute_meter.clone(), + loader_id: &bpf_loader::id(), + enforce_aligned_host_addrs: true, + allow_native_ids: true, + check_seed_length: true, + }; + call_program_address_common(seeds, program_id, &mut syscall) + } + + #[test] + fn test_create_program_address() { + // These tests duplicate the direct tests in solana_program::pubkey + + let program_id = Pubkey::from_str("BPFLoaderUpgradeab1e11111111111111111111111").unwrap(); + + let exceeded_seed = &[127; MAX_SEED_LEN + 1]; + let result = create_program_address(&[exceeded_seed], &program_id, 1); + assert_eq!( + result, + Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()) + ); + assert_eq!( + create_program_address(&[b"short_seed", exceeded_seed], &program_id, 1), + Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()) + ); + let max_seed = &[0; MAX_SEED_LEN]; + assert!(create_program_address(&[max_seed], &program_id, 1).is_ok()); + let exceeded_seeds: &[&[u8]] = &[ + &[1], + &[2], + &[3], + &[4], + &[5], + &[6], + &[7], + &[8], + &[9], + &[10], + &[11], + &[12], + &[13], + &[14], + &[15], + &[16], + ]; + assert!(create_program_address(exceeded_seeds, &program_id, 1).is_ok()); + let max_seeds: &[&[u8]] = &[ + &[1], + &[2], + &[3], + &[4], + &[5], + &[6], + &[7], + &[8], + &[9], + &[10], + &[11], + &[12], + &[13], + &[14], + &[15], + &[16], + &[17], + ]; + assert_eq!( + create_program_address(max_seeds, &program_id, 1), + Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()) + ); + assert_eq!( + create_program_address(&[b"", &[1]], &program_id, 0), + Err( + SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) + .into() + ) + ); + assert_eq!( + create_program_address(&[b"", &[1]], &program_id, 1), + Ok("BwqrghZA2htAcqq8dzP1WDAhTXYTYWj7CHxF5j7TDBAe" + .parse() + .unwrap()) + ); + assert_eq!( + create_program_address(&["☉".as_ref(), &[0]], &program_id, 1), + Ok("13yWmRpaTR4r5nAktwLqMpRNr28tnVUZw26rTvPSSB19" + .parse() + .unwrap()) + ); + assert_eq!( + create_program_address(&[b"Talking", b"Squirrels"], &program_id, 1), + Ok("2fnQrngrQT4SeLcdToJAD96phoEjNL2man2kfRLCASVk" + .parse() + .unwrap()) + ); + let public_key = Pubkey::from_str("SeedPubey1111111111111111111111111111111111").unwrap(); + assert_eq!( + create_program_address(&[public_key.as_ref(), &[1]], &program_id, 1), + Ok("976ymqVnfE32QFe6NfGDctSvVa36LWnvYxhU6G2232YL" + .parse() + .unwrap()) + ); + assert_ne!( + create_program_address(&[b"Talking", b"Squirrels"], &program_id, 1).unwrap(), + create_program_address(&[b"Talking"], &program_id, 1).unwrap(), + ); + } + + #[test] + fn test_find_program_address() { + for _ in 0..1_000 { + let program_id = Pubkey::new_unique(); + let (address, bump_seed) = + try_find_program_address(&[b"Lil'", b"Bits"], &program_id, 100).unwrap(); + assert_eq!( + address, + create_program_address(&[b"Lil'", b"Bits", &[bump_seed]], &program_id, 1).unwrap() + ); + } + + let program_id = Pubkey::from_str("BPFLoaderUpgradeab1e11111111111111111111111").unwrap(); + let max_tries = 256; // one per seed + let seeds: &[&[u8]] = &[b""]; + let (_, bump_seed) = try_find_program_address(seeds, &program_id, max_tries).unwrap(); + let remaining = 256 - bump_seed as u64; + let _ = try_find_program_address(seeds, &program_id, remaining).unwrap(); + assert_eq!( + try_find_program_address(seeds, &program_id, remaining - 1), + Err( + SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) + .into() + ) + ); + let exceeded_seed = &[127; MAX_SEED_LEN + 1]; + assert_eq!( + try_find_program_address(&[exceeded_seed], &program_id, max_tries - 1), + Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()) + ); + let exceeded_seeds: &[&[u8]] = &[ + &[1], + &[2], + &[3], + &[4], + &[5], + &[6], + &[7], + &[8], + &[9], + &[10], + &[11], + &[12], + &[13], + &[14], + &[15], + &[16], + &[17], + ]; + assert_eq!( + try_find_program_address(exceeded_seeds, &program_id, max_tries - 1), + Err(SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded).into()) + ); + } } diff --git a/sdk/program/src/pubkey.rs b/sdk/program/src/pubkey.rs index bd2880ed95..fb35cdcb3b 100644 --- a/sdk/program/src/pubkey.rs +++ b/sdk/program/src/pubkey.rs @@ -497,6 +497,43 @@ mod tests { fn test_create_program_address() { let exceeded_seed = &[127; MAX_SEED_LEN + 1]; let max_seed = &[0; MAX_SEED_LEN]; + let exceeded_seeds: &[&[u8]] = &[ + &[1], + &[2], + &[3], + &[4], + &[5], + &[6], + &[7], + &[8], + &[9], + &[10], + &[11], + &[12], + &[13], + &[14], + &[15], + &[16], + &[17], + ]; + let max_seeds: &[&[u8]] = &[ + &[1], + &[2], + &[3], + &[4], + &[5], + &[6], + &[7], + &[8], + &[9], + &[10], + &[11], + &[12], + &[13], + &[14], + &[15], + &[16], + ]; let program_id = Pubkey::from_str("BPFLoaderUpgradeab1e11111111111111111111111").unwrap(); let public_key = Pubkey::from_str("SeedPubey1111111111111111111111111111111111").unwrap(); @@ -509,6 +546,11 @@ mod tests { Err(PubkeyError::MaxSeedLengthExceeded) ); assert!(Pubkey::create_program_address(&[max_seed], &program_id).is_ok()); + assert_eq!( + Pubkey::create_program_address(exceeded_seeds, &program_id), + Err(PubkeyError::MaxSeedLengthExceeded) + ); + assert!(Pubkey::create_program_address(max_seeds, &program_id).is_ok()); assert_eq!( Pubkey::create_program_address(&[b"", &[1]], &program_id), Ok("BwqrghZA2htAcqq8dzP1WDAhTXYTYWj7CHxF5j7TDBAe" diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 49d8f21725..430f699426 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -195,6 +195,10 @@ pub mod allow_native_ids { solana_sdk::declare_id!("GVnDbNkECwrzLM7aVBGWpBYo3yH1ACaXB4ottNX8pedZ"); } +pub mod check_seed_length { + solana_sdk::declare_id!("8HYXgkoKGreAMA3MfJkdjbKNVbfZRQP3jqFpa7iqN4v7"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -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"), (allow_native_ids::id(), "allow native program ids in program derived addresses"), + (check_seed_length::id(), "Check program address seed lengths"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()