From 778b2adbeaa35703ee2e958720bcff60e18db292 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 7 Sep 2021 21:37:24 -0700 Subject: [PATCH] Check seed length before trying to cal program address (#19699) --- Cargo.lock | 1 + programs/bpf_loader/Cargo.toml | 1 + programs/bpf_loader/src/syscalls.rs | 330 ++++++++++++++++++++++++++-- sdk/program/src/pubkey.rs | 42 ++++ sdk/src/feature_set.rs | 5 + 5 files changed, 362 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb495ba86c..828e142ca9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4293,6 +4293,7 @@ version = "1.8.0" dependencies = [ "bincode", "byteorder", + "curve25519-dalek 3.2.0", "libsecp256k1 0.6.0", "log 0.4.14", "num-derive", diff --git a/programs/bpf_loader/Cargo.toml b/programs/bpf_loader/Cargo.toml index 4c84bf1286..3fe7bd8578 100644 --- a/programs/bpf_loader/Cargo.toml +++ b/programs/bpf_loader/Cargo.toml @@ -28,6 +28,7 @@ solana_rbpf = "=0.2.14" thiserror = "1.0" [dev-dependencies] +curve25519-dalek = "3.0.0" rand = "0.7.3" rustversion = "1.0.5" solana-runtime = { path = "../../runtime", version = "=1.8.0" } diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index fb64b7b64d..c09727475d 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -21,9 +21,10 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - 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, + allow_native_ids, blake3_syscall_enabled, check_seed_length, + 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, @@ -248,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: compute_budget.create_program_address_units, @@ -255,6 +257,7 @@ pub fn bind_syscall_context_objects<'a>( loader_id, enforce_aligned_host_addrs, allow_native_ids, + check_seed_length, }), None, )?; @@ -265,6 +268,7 @@ pub fn bind_syscall_context_objects<'a>( loader_id, enforce_aligned_host_addrs, allow_native_ids, + check_seed_length, }), None, )?; @@ -797,13 +801,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, @@ -818,6 +823,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, @@ -872,6 +880,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( @@ -884,19 +893,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); @@ -932,6 +948,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( @@ -944,14 +961,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 ); @@ -962,7 +984,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) = @@ -995,6 +1019,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); } @@ -2538,6 +2565,12 @@ mod tests { }; } + #[allow(dead_code)] + struct MockSlice { + pub vm_addr: u64, + pub len: usize, + } + #[test] fn test_translate() { const START: u64 = 0x100000000; @@ -3250,17 +3283,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: 0x300000000, + vm_addr: 0x300000000, len: bytes1.len(), }; let mock_slice2 = MockSlice { - addr: 0x400000000, + vm_addr: 0x400000000, len: bytes2.len(), }; let bytes_to_hash = [mock_slice1, mock_slice2]; @@ -3288,14 +3316,14 @@ mod tests { }, MemoryRegion { host_addr: bytes1.as_ptr() as *const _ as u64, - vm_addr: bytes_to_hash[0].addr, + vm_addr: bytes_to_hash[0].vm_addr, len: bytes1.len() as u64, vm_gap_shift: 63, is_writable: false, }, MemoryRegion { host_addr: bytes2.as_ptr() as *const _ as u64, - vm_addr: bytes_to_hash[1].addr, + vm_addr: bytes_to_hash[1].vm_addr, len: bytes2.len() as u64, vm_gap_shift: 63, is_writable: false, @@ -3564,4 +3592,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 3c04261140..ab0e08227d 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -203,6 +203,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 = [ @@ -249,6 +253,7 @@ lazy_static! { (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"), + (check_seed_length::id(), "Check program address seed lengths"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()