diff --git a/p-token/src/entrypoint-runtime-verification.rs b/p-token/src/entrypoint-runtime-verification.rs index 36d6487c..73f2202b 100644 --- a/p-token/src/entrypoint-runtime-verification.rs +++ b/p-token/src/entrypoint-runtime-verification.rs @@ -1195,6 +1195,12 @@ pub fn test_process_transfer_multisig( let old_src_delgated_amount = src_old.delegated_amount(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + // avoids potential overflow in destination account. assuming global supply bound by u64 + if accounts[0] != accounts[1] && dst_initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = process_transfer(accounts, instruction_data); @@ -1455,6 +1461,18 @@ pub fn test_process_mint_to_multisig( let dst_init_state = dst_old.account_state(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + { + // Do not execute if adding to the account balance would overflow. + // shared::mint_to.rs,L68 is based on the assumption that initial_amount <= + // mint.supply and therefore cannot overflow because the minting itself + // would already error out. + let amount = unsafe { u64::from_le_bytes(*(instruction_data.as_ptr() as *const [u8; 8])) }; + if initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + } + //-Process Instruction----------------------------------------------------- let result = process_mint_to(accounts, instruction_data); @@ -1684,13 +1702,21 @@ pub fn test_process_burn_multisig( let src_mint = src_old.mint; let src_owned_sys_inc = src_old.is_owned_by_system_program_or_incinerator(); let src_owner = src_old.owner; + let src_info_owner = accounts[0].owner(); let old_src_delgate = src_old.delegate().cloned(); let old_src_delgated_amount = src_old.delegated_amount(); let mint_initialised = mint_old.is_initialized(); let mint_init_supply = mint_old.supply(); - let mint_owner = *accounts[1].owner(); + let mint_info_owner = *accounts[1].owner(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + // accoutn.amount() <= mint.supply(), account.delegated_amount() <= account.amount() + // otherwise processing could lead to overflows, see processor::shared::burn,L83 + if !(src_init_amount <= mint_init_supply && old_src_delgated_amount <= src_init_amount) { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = process_burn(accounts, instruction_data); @@ -1747,9 +1773,9 @@ pub fn test_process_burn_multisig( } } - if amount == 0 && src_owner != pinocchio_token_interface::program::ID { + if amount == 0 && *src_info_owner != pinocchio_token_interface::program::ID { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) - } else if amount == 0 && mint_owner != pinocchio_token_interface::program::ID { + } else if amount == 0 && mint_info_owner != pinocchio_token_interface::program::ID { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) } else { let src_new = get_account(&accounts[0]); @@ -1758,11 +1784,19 @@ pub fn test_process_burn_multisig( assert!(result.is_ok()); // Delegate updates - if old_src_delgate.is_some() && *accounts[2].key() == old_src_delgate.unwrap() { - assert_eq!(src_new.delegated_amount(), old_src_delgated_amount - amount); + let new_src_delegate = src_new.delegate().cloned(); + let new_src_delegated_amount = src_new.delegated_amount(); + if !src_owned_sys_inc + && old_src_delgate.is_some() + && *accounts[2].key() == old_src_delgate.unwrap() + { + assert_eq!(new_src_delegated_amount, old_src_delgated_amount - amount); if old_src_delgated_amount - amount == 0 { - assert_eq!(src_new.delegate(), None); + assert_eq!(new_src_delegate, None); } + } else { + assert_eq!(old_src_delgate, new_src_delegate); + assert_eq!(old_src_delgated_amount, new_src_delegated_amount); } } } @@ -1910,19 +1944,25 @@ pub fn test_process_close_account_multisig(accounts: &[AccountInfo; 4]) -> Progr } else if accounts[1].key() != &INCINERATOR_ID { assert_eq!(result, Err(ProgramError::InvalidAccountData)); return result; - } else if u64::MAX - src_init_lamports < dst_init_lamports { + } + if dst_init_lamports.checked_add(src_init_lamports).is_none() { assert_eq!(result, Err(ProgramError::Custom(14))); return result; } + assert!(result.is_ok()); // Validate owner falls through to here if no error - assert_eq!(accounts[0].lamports(), 0); assert_eq!( accounts[1].lamports(), dst_init_lamports + src_init_lamports ); - assert_eq!(accounts[0].data_len(), 0); // TODO: More sol_memset stuff? - assert!(result.is_ok()); + #[cfg(any(target_os = "solana", target_arch = "bpf"))] + { + // Solana-RT only syscall + assert_eq!(*accounts[0].owner(), [0; 32]); + assert_eq!(accounts[0].lamports(), 0); + assert_eq!(accounts[0].data_len(), 0); + } } result } @@ -2132,6 +2172,12 @@ pub fn test_process_transfer_checked_multisig( let mint_initialised = get_mint(&accounts[1]).is_initialized(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[3]).is_initialized()); + #[cfg(feature = "assumptions")] + // avoids potential overflow in destination account. assuming global supply bound by u64 + if accounts[0] != accounts[2] && dst_initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = process_transfer_checked(accounts, instruction_data); @@ -2245,7 +2291,7 @@ pub fn test_process_transfer_checked_multisig( if src_new.is_native() { assert_eq!(accounts[0].lamports(), src_initial_lamports - amount); - assert_eq!(accounts[1].lamports(), dst_initial_lamports + amount); + assert_eq!(accounts[2].lamports(), dst_initial_lamports + amount); } } @@ -2420,14 +2466,22 @@ pub fn test_process_burn_checked_multisig( let src_mint = src_old.mint; let src_owned_sys_inc = src_old.is_owned_by_system_program_or_incinerator(); let src_owner = src_old.owner; + let src_info_owner = accounts[0].owner(); let old_src_delgate = src_old.delegate().cloned(); let old_src_delgated_amount = src_old.delegated_amount(); let mint_initialised = mint_old.is_initialized(); let mint_init_supply = mint_old.supply(); let mint_decimals = mint_old.decimals; - let mint_owner = *accounts[1].owner(); + let mint_info_owner = *accounts[1].owner(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + // accoutn.amount() <= mint.supply(), account.delegated_amount() <= account.amount() + // otherwise processing could lead to overflows, see processor::shared::burn,L83 + if !(src_init_amount <= mint_init_supply && old_src_delgated_amount <= src_init_amount) { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = process_burn_checked(accounts, instruction_data); @@ -2486,9 +2540,9 @@ pub fn test_process_burn_checked_multisig( } } - if amount == 0 && src_owner != pinocchio_token_interface::program::ID { + if amount == 0 && *src_info_owner != pinocchio_token_interface::program::ID { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) - } else if amount == 0 && mint_owner != pinocchio_token_interface::program::ID { + } else if amount == 0 && mint_info_owner != pinocchio_token_interface::program::ID { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) } else { let src_new = get_account(&accounts[0]); @@ -2497,11 +2551,19 @@ pub fn test_process_burn_checked_multisig( assert!(result.is_ok()); // Delegate updates - if old_src_delgate.is_some() && *accounts[2].key() == old_src_delgate.unwrap() { - assert_eq!(src_new.delegated_amount(), old_src_delgated_amount - amount); + let new_src_delegate = src_new.delegate().cloned(); + let new_src_delegated_amount = src_new.delegated_amount(); + if !src_owned_sys_inc + && old_src_delgate.is_some() + && *accounts[2].key() == old_src_delgate.unwrap() + { + assert_eq!(new_src_delegated_amount, old_src_delgated_amount - amount); if old_src_delgated_amount - amount == 0 { - assert_eq!(src_new.delegate(), None); + assert_eq!(new_src_delegate, None); } + } else { + assert_eq!(old_src_delgate, new_src_delegate); + assert_eq!(old_src_delgated_amount, new_src_delegated_amount); } } } @@ -4095,6 +4157,18 @@ fn test_process_mint_to_checked_multisig( let dst_init_state = dst_old.account_state(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + { + // Do not execute if adding to the account balance would overflow. + // shared::mint_to.rs,L68 is based on the assumption that initial_amount <= + // mint.supply and therefore cannot overflow because the minting itself + // would already error out. + let amount = unsafe { u64::from_le_bytes(*(instruction_data.as_ptr() as *const [u8; 8])) }; + if initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + } + //-Process Instruction----------------------------------------------------- let result = process_mint_to_checked(accounts, instruction_data); diff --git a/program/src/entrypoint-runtime-verification.rs b/program/src/entrypoint-runtime-verification.rs index 2dbe6326..8657f48a 100644 --- a/program/src/entrypoint-runtime-verification.rs +++ b/program/src/entrypoint-runtime-verification.rs @@ -971,15 +971,22 @@ fn test_process_burn_checked_multisig( let src_mint = get_account(&accounts[0]).mint(); let src_owned_sys_inc = get_account(&accounts[0]).is_owned_by_system_program_or_incinerator(); let src_owner = get_account(&accounts[0]).owner(); + let src_info_owner = accounts[0].owner; let old_src_delgate = get_account(&accounts[0]).delegate().cloned(); let old_src_delgated_amount = get_account(&accounts[0]).delegated_amount(); let mint_initialised = get_mint(&accounts[1]).is_initialized(); let mint_init_supply = get_mint(&accounts[1]).supply(); let mint_decimals = get_mint(&accounts[1]).decimals(); - let mint_owner = *accounts[1].owner; + let mint_info_owner = *accounts[1].owner; let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); let tx_signers: &[AccountInfo] = &accounts[3..]; + #[cfg(feature = "assumptions")] + // Assume balances stay within u64 so processing cannot overflow + if !(src_init_amount <= mint_init_supply && old_src_delgated_amount <= src_init_amount) { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -1036,21 +1043,30 @@ fn test_process_burn_checked_multisig( } } - if amount == 0 && src_owner != crate::id() { + if amount == 0 && *src_info_owner != crate::id() { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) - } else if amount == 0 && mint_owner != crate::id() { + } else if amount == 0 && mint_info_owner != crate::id() { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) } else { - assert!(get_account(&accounts[0]).amount() == src_init_amount - amount); + let src_new = get_account(&accounts[0]); + assert!(src_new.amount() == src_init_amount - amount); assert!(get_mint(&accounts[1]).supply() == mint_init_supply - amount); assert!(result.is_ok()); // Delegate updates - if old_src_delgate.is_some() && *accounts[2].key == old_src_delgate.unwrap() { - assert_eq!(get_account(&accounts[0]).delegated_amount(), old_src_delgated_amount - amount); + let new_src_delegate = src_new.delegate().cloned(); + let new_src_delegated_amount = src_new.delegated_amount(); + if !src_owned_sys_inc + && old_src_delgate.is_some() + && *accounts[2].key == old_src_delgate.unwrap() + { + assert_eq!(new_src_delegated_amount, old_src_delgated_amount - amount); if old_src_delgated_amount - amount == 0 { - assert_eq!(get_account(&accounts[0]).delegate(), None); + assert_eq!(new_src_delegate, None); } + } else { + assert_eq!(old_src_delgate, new_src_delegate); + assert_eq!(old_src_delgated_amount, new_src_delegated_amount); } } } @@ -1256,6 +1272,18 @@ fn test_process_mint_to_checked_multisig( let dst_init_state = get_account(&accounts[1]).account_state(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + { + // Do not execute if adding to the account balance would overflow. + // shared::mint_to.rs,L68 is based on the assumption that initial_amount <= + // mint.supply and therefore cannot overflow because the minting itself + // would already error out. + let amount = unsafe { u64::from_le_bytes(*(instruction_data.as_ptr() as *const [u8; 8])) }; + if initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -1376,14 +1404,21 @@ fn test_process_burn_multisig( let src_mint = get_account(&accounts[0]).mint(); let src_owned_sys_inc = get_account(&accounts[0]).is_owned_by_system_program_or_incinerator(); let src_owner = get_account(&accounts[0]).owner(); + let src_info_owner = accounts[0].owner; let old_src_delgate = get_account(&accounts[0]).delegate().cloned(); let old_src_delgated_amount = get_account(&accounts[0]).delegated_amount(); let mint_initialised = get_mint(&accounts[1]).is_initialized(); let mint_init_supply = get_mint(&accounts[1]).supply(); - let mint_owner = *accounts[1].owner; + let mint_info_owner = *accounts[1].owner; let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); let tx_signers: &[AccountInfo] = &accounts[3..]; + #[cfg(feature = "assumptions")] + // Assume balances stay within u64 so processing cannot overflow + if !(src_init_amount <= mint_init_supply && old_src_delgated_amount <= src_init_amount) { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -1438,21 +1473,30 @@ fn test_process_burn_multisig( } } - if amount == 0 && src_owner != crate::id() { + if amount == 0 && *src_info_owner != crate::id() { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) - } else if amount == 0 && mint_owner != crate::id() { + } else if amount == 0 && mint_info_owner != crate::id() { assert_eq!(result, Err(ProgramError::IncorrectProgramId)) } else { - assert!(get_account(&accounts[0]).amount() == src_init_amount - amount); + let src_new = get_account(&accounts[0]); + assert!(src_new.amount() == src_init_amount - amount); assert!(get_mint(&accounts[1]).supply() == mint_init_supply - amount); assert!(result.is_ok()); // Delegate updates - if old_src_delgate.is_some() && *accounts[2].key == old_src_delgate.unwrap() { - assert_eq!(get_account(&accounts[0]).delegated_amount(), old_src_delgated_amount - amount); + let new_src_delegate = src_new.delegate().cloned(); + let new_src_delegated_amount = src_new.delegated_amount(); + if !src_owned_sys_inc + && old_src_delgate.is_some() + && *accounts[2].key == old_src_delgate.unwrap() + { + assert_eq!(new_src_delegated_amount, old_src_delgated_amount - amount); if old_src_delgated_amount - amount == 0 { - assert_eq!(get_account(&accounts[0]).delegate(), None); + assert_eq!(new_src_delegate, None); } + } else { + assert_eq!(old_src_delgate, new_src_delegate); + assert_eq!(old_src_delgated_amount, new_src_delegated_amount); } } } @@ -1650,6 +1694,12 @@ fn test_process_transfer_multisig( let old_src_delgated_amount = get_account(&accounts[0]).delegated_amount(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + // avoids potential overflow in destination account. assuming global supply bound by u64 + if accounts[0] != accounts[1] && dst_initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -1792,6 +1842,12 @@ fn test_process_transfer_checked_multisig( let mint_initialised = get_mint(&accounts[1]).is_initialized(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[3]).is_initialized()); + #[cfg(feature = "assumptions")] + // avoids potential overflow in destination account. assuming global supply bound by u64 + if accounts[0] != accounts[2] && dst_initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -1904,7 +1960,7 @@ fn test_process_transfer_checked_multisig( if src_new.is_native() { assert_eq!(accounts[0].lamports(), src_initial_lamports - amount); - assert_eq!(accounts[1].lamports(), dst_initial_lamports + amount); + assert_eq!(accounts[2].lamports(), dst_initial_lamports + amount); } } @@ -2919,6 +2975,16 @@ fn test_process_mint_to_multisig( let dst_init_state = get_account(&accounts[1]).account_state(); let maybe_multisig_is_initialised = Some(get_multisig(&accounts[2]).is_initialized()); + #[cfg(feature = "assumptions")] + { + // Skip cases that would overflow the destination balance assuming total supply fits in u64 + let amount = + u64::from_le_bytes([instruction_data[0], instruction_data[1], instruction_data[2], instruction_data[3], instruction_data[4], instruction_data[5], instruction_data[6], instruction_data[7]]); + if initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator); @@ -3296,26 +3362,36 @@ fn test_process_close_account_multisig( return result; } else { if !src_owned_sys_inc { + // Validate Owner inner_test_validate_owner( - &authority, - &accounts[2], - tx_signers, - maybe_multisig_is_initialised.clone(), + &authority, // expected_owner + &accounts[2], // owner_account_info + &accounts[3..], // tx_signers + maybe_multisig_is_initialised, result.clone(), )?; } else if accounts[1].key != &INCINERATOR_ID { assert_eq!(result, Err(ProgramError::InvalidAccountData)); return result; - } else if dst_init_lamports.checked_add(src_init_lamports).is_none() { + } + if dst_init_lamports.checked_add(src_init_lamports).is_none() { assert_eq!(result, Err(ProgramError::Custom(14))); return result; } + assert!(result.is_ok()); // Validate owner falls through to here if no error - assert_eq!(accounts[1].lamports(), dst_init_lamports + src_init_lamports); - assert_eq!(accounts[0].lamports(), 0); - assert_eq!(accounts[0].data_len(), 0); // TODO: More sol_memset stuff? - assert!(result.is_ok()); + assert_eq!( + accounts[1].lamports(), + dst_init_lamports + src_init_lamports + ); + #[cfg(any(target_os = "solana", target_arch = "bpf"))] + { + // Solana-RT only syscall + assert_eq!(*accounts[0].owner, Pubkey::from([0; 32])); + assert_eq!(accounts[0].lamports(), 0); + assert_eq!(accounts[0].data_len(), 0); + } } // Ensure instruction_data was not mutated @@ -3944,6 +4020,18 @@ fn test_process_mint_to_checked( let dst_init_state = get_account(&accounts[1]).account_state(); let maybe_multisig_is_initialised = None; + #[cfg(feature = "assumptions")] + { + // Do not execute if adding to the account balance would overflow. + // shared::mint_to.rs,L68 is based on the assumption that initial_amount <= + // mint.supply and therefore cannot overflow because the minting itself + // would already error out. + let amount = unsafe { u64::from_le_bytes(*(instruction_data.as_ptr() as *const [u8; 8])) }; + if initial_amount.checked_add(amount).is_none() { + return Err(ProgramError::Custom(99)); + } + } + //-Process Instruction----------------------------------------------------- let result = Processor::process(program_id, accounts, instruction_data_with_discriminator);