Owner and sysvar check hardening (#154)

* owner and sysvar check hardening
This commit is contained in:
Hendrik Hofstadt 2021-01-12 23:21:30 +01:00 committed by GitHub
parent ad5950ffe7
commit 274bb7c97d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 53 deletions

View File

@ -106,6 +106,12 @@ pub enum Error {
/// Insufficient fees
#[error("InsufficientFees")]
InsufficientFees,
/// Invalid owner
#[error("InvalidOwner")]
InvalidOwner,
/// Invalid Sysvar
#[error("InvalidSysvar")]
InvalidSysvar,
}
impl From<Error> for ProgramError {

View File

@ -11,35 +11,37 @@ impl PrintProgramError for Error {
E: 'static + std::error::Error + DecodeError<E> + PrintProgramError + FromPrimitive,
{
match self {
Error::ExpectedToken => info!("Error: ExpectedToken"),
Error::ExpectedAccount => info!("Error: ExpectedAccount"),
Error::ExpectedBridge => info!("Error: ExpectedBridge"),
Error::ExpectedGuardianSet => info!("Error: ExpectedGuardianSet"),
Error::ExpectedWrappedAssetMeta => info!("Error: ExpectedWrappedAssetMeta"),
Error::UninitializedState => info!("Error: State is unititialized"),
Error::InvalidProgramAddress => info!("Error: InvalidProgramAddress"),
Error::InvalidVAAFormat => info!("Error: InvalidVAAFormat"),
Error::InvalidVAAAction => info!("Error: InvalidVAAAction"),
Error::InvalidVAASignature => info!("Error: InvalidVAASignature"),
Error::AlreadyExists => info!("Error: AlreadyExists"),
Error::InvalidDerivedAccount => info!("Error: InvalidDerivedAccount"),
Error::TokenMintMismatch => info!("Error: TokenMintMismatch"),
Error::WrongMintOwner => info!("Error: WrongMintOwner"),
Error::WrongTokenAccountOwner => info!("Error: WrongTokenAccountOwner"),
Error::ParseFailed => info!("Error: ParseFailed"),
Error::GuardianSetExpired => info!("Error: GuardianSetExpired"),
Error::VAAClaimed => info!("Error: VAAClaimed"),
Error::WrongBridgeOwner => info!("Error: WrongBridgeOwner"),
Error::OldGuardianSet => info!("Error: OldGuardianSet"),
Error::GuardianIndexNotIncreasing => info!("Error: GuardianIndexNotIncreasing"),
Error::ExpectedTransferOutProposal => info!("Error: ExpectedTransferOutProposal"),
Error::VAAProposalMismatch => info!("Error: VAAProposalMismatch"),
Error::SameChainTransfer => info!("Error: SameChainTransfer"),
Error::VAATooLong => info!("Error: VAATooLong"),
Error::CannotWrapNative => info!("Error: CannotWrapNative"),
Error::VAAAlreadySubmitted => info!("Error: VAAAlreadySubmitted"),
Error::GuardianSetMismatch => info!("Error: GuardianSetMismatch"),
Error::InsufficientFees => info!("Error: InsufficientFees"),
Error::ExpectedToken => msg!("Error: ExpectedToken"),
Error::ExpectedAccount => msg!("Error: ExpectedAccount"),
Error::ExpectedBridge => msg!("Error: ExpectedBridge"),
Error::ExpectedGuardianSet => msg!("Error: ExpectedGuardianSet"),
Error::ExpectedWrappedAssetMeta => msg!("Error: ExpectedWrappedAssetMeta"),
Error::UninitializedState => msg!("Error: State is unititialized"),
Error::InvalidProgramAddress => msg!("Error: InvalidProgramAddress"),
Error::InvalidVAAFormat => msg!("Error: InvalidVAAFormat"),
Error::InvalidVAAAction => msg!("Error: InvalidVAAAction"),
Error::InvalidVAASignature => msg!("Error: InvalidVAASignature"),
Error::AlreadyExists => msg!("Error: AlreadyExists"),
Error::InvalidDerivedAccount => msg!("Error: InvalidDerivedAccount"),
Error::TokenMintMismatch => msg!("Error: TokenMintMismatch"),
Error::WrongMintOwner => msg!("Error: WrongMintOwner"),
Error::WrongTokenAccountOwner => msg!("Error: WrongTokenAccountOwner"),
Error::ParseFailed => msg!("Error: ParseFailed"),
Error::GuardianSetExpired => msg!("Error: GuardianSetExpired"),
Error::VAAClaimed => msg!("Error: VAAClaimed"),
Error::WrongBridgeOwner => msg!("Error: WrongBridgeOwner"),
Error::OldGuardianSet => msg!("Error: OldGuardianSet"),
Error::GuardianIndexNotIncreasing => msg!("Error: GuardianIndexNotIncreasing"),
Error::ExpectedTransferOutProposal => msg!("Error: ExpectedTransferOutProposal"),
Error::VAAProposalMismatch => msg!("Error: VAAProposalMismatch"),
Error::SameChainTransfer => msg!("Error: SameChainTransfer"),
Error::VAATooLong => msg!("Error: VAATooLong"),
Error::CannotWrapNative => msg!("Error: CannotWrapNative"),
Error::VAAAlreadySubmitted => msg!("Error: VAAAlreadySubmitted"),
Error::GuardianSetMismatch => msg!("Error: GuardianSetMismatch"),
Error::InsufficientFees => msg!("Error: InsufficientFees"),
Error::InvalidOwner => msg!("Error: InvalidOwner"),
Error::InvalidSysvar => msg!("Error: InvalidSysvar"),
}
}
}

View File

@ -59,7 +59,7 @@ impl Bridge {
let instruction = BridgeInstruction::deserialize(input)?;
match instruction {
Initialize(payload) => {
info!("Instruction: Initialize");
msg!("Instruction: Initialize");
Self::process_initialize(
program_id,
accounts,
@ -69,7 +69,7 @@ impl Bridge {
)
}
TransferOut(p) => {
info!("Instruction: TransferOut");
msg!("Instruction: TransferOut");
if p.asset.chain == CHAIN_ID_SOLANA {
Self::process_transfer_native_out(program_id, accounts, &p)
@ -78,23 +78,23 @@ impl Bridge {
}
}
PostVAA(vaa_body) => {
info!("Instruction: PostVAA");
msg!("Instruction: PostVAA");
let vaa = VAA::deserialize(&vaa_body)?;
Self::process_vaa(program_id, accounts, vaa_body, &vaa)
}
PokeProposal() => {
info!("Instruction: PokeProposal");
msg!("Instruction: PokeProposal");
Self::process_poke(program_id, accounts)
}
VerifySignatures(p) => {
info!("Instruction: VerifySignatures");
msg!("Instruction: VerifySignatures");
Self::process_verify_signatures(program_id, accounts, &p)
}
CreateWrapped(meta) => {
info!("Instruction: CreateWrapped");
msg!("Instruction: CreateWrapped");
Self::process_create_wrapped(program_id, accounts, &meta)
}
_ => panic!(""),
@ -176,7 +176,7 @@ impl Bridge {
/// Transfers a wrapped asset out
pub fn process_poke(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramResult {
let account_info_iter = &mut accounts.iter();
let proposal_info = next_account_info(account_info_iter)?;
let proposal_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let mut transfer_data = proposal_info.try_borrow_mut_data()?;
let mut proposal: &mut TransferOutProposal = Self::unpack(&mut transfer_data)?;
@ -200,11 +200,15 @@ impl Bridge {
next_account_info(account_info_iter)?; // Bridge program
next_account_info(account_info_iter)?; // System program
let instruction_accounts = next_account_info(account_info_iter)?;
let bridge_info = next_account_info(account_info_iter)?;
let bridge_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let sig_info = next_account_info(account_info_iter)?;
let guardian_set_info = next_account_info(account_info_iter)?;
let guardian_set_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let payer_info = next_account_info(account_info_iter)?;
if *instruction_accounts.key != solana_program::sysvar::instructions::id() {
return Err(Error::InvalidSysvar.into());
}
let guardian_data = guardian_set_info.try_borrow_data()?;
let guardian_set: &GuardianSet = Self::unpack_immutable(&guardian_data)?;
@ -369,7 +373,7 @@ impl Bridge {
accounts: &[AccountInfo],
t: &TransferOutPayload,
) -> ProgramResult {
info!("wrapped transfer out");
msg!("wrapped transfer out");
let account_info_iter = &mut accounts.iter();
next_account_info(account_info_iter)?; // Bridge program
next_account_info(account_info_iter)?; // System program
@ -378,7 +382,7 @@ impl Bridge {
let clock_info = next_account_info(account_info_iter)?;
let instructions_info = next_account_info(account_info_iter)?;
let sender_account_info = next_account_info(account_info_iter)?;
let bridge_info = next_account_info(account_info_iter)?;
let bridge_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let transfer_info = next_account_info(account_info_iter)?;
let mint_info = next_account_info(account_info_iter)?;
let payer_info = next_account_info(account_info_iter)?;
@ -389,6 +393,10 @@ impl Bridge {
let mint = Bridge::mint_deserialize(mint_info)?;
let clock = Clock::from_account_info(clock_info)?;
if *instructions_info.key != solana_program::sysvar::instructions::id() {
return Err(Error::InvalidSysvar.into());
}
// Fee handling
let fee = Self::transfer_fee();
Self::check_fees(instructions_info, bridge_info, fee)?;
@ -469,7 +477,7 @@ impl Bridge {
accounts: &[AccountInfo],
t: &TransferOutPayload,
) -> ProgramResult {
info!("native transfer out");
msg!("native transfer out");
let account_info_iter = &mut accounts.iter();
next_account_info(account_info_iter)?; // Bridge program
next_account_info(account_info_iter)?; // System program
@ -478,7 +486,7 @@ impl Bridge {
let clock_info = next_account_info(account_info_iter)?;
let instructions_info = next_account_info(account_info_iter)?;
let sender_account_info = next_account_info(account_info_iter)?;
let bridge_info = next_account_info(account_info_iter)?;
let bridge_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let transfer_info = next_account_info(account_info_iter)?;
let mint_info = next_account_info(account_info_iter)?;
let payer_info = next_account_info(account_info_iter)?;
@ -551,7 +559,12 @@ impl Bridge {
return Err(Error::WrongTokenAccountOwner.into());
}
info!("transferring");
// Check that the source is not the custody account
if custody_info.key == sender_account_info.key {
return Err(Error::WrongTokenAccountOwner.into());
}
msg!("transferring");
// Transfer tokens to custody - This also checks that custody mint = mint
Bridge::token_transfer_caller(
program_id,
@ -667,10 +680,10 @@ impl Bridge {
next_account_info(account_info_iter)?; // System program
next_account_info(account_info_iter)?; // Rent sysvar
let clock_info = next_account_info(account_info_iter)?;
let bridge_info = next_account_info(account_info_iter)?;
let guardian_set_info = next_account_info(account_info_iter)?;
let bridge_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let guardian_set_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let claim_info = next_account_info(account_info_iter)?;
let sig_info = next_account_info(account_info_iter)?;
let sig_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let payer_info = next_account_info(account_info_iter)?;
let clock = Clock::from_account_info(clock_info)?;
@ -946,8 +959,8 @@ impl Bridge {
vaa_data: VAAData,
sig_account: &Pubkey,
) -> ProgramResult {
info!("posting VAA");
let proposal_info = next_account_info(account_info_iter)?;
msg!("posting VAA");
let proposal_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
// Check whether the proposal was derived correctly
let expected_proposal = Bridge::derive_transfer_id(
@ -994,12 +1007,12 @@ impl Bridge {
accounts: &[AccountInfo],
a: &AssetMeta,
) -> ProgramResult {
info!("create wrapped");
msg!("create wrapped");
let account_info_iter = &mut accounts.iter();
next_account_info(account_info_iter)?; // System program
next_account_info(account_info_iter)?; // Token program
next_account_info(account_info_iter)?; // Rent sysvar
let bridge_info = next_account_info(account_info_iter)?;
let bridge_info = Self::next_account_info_with_owner(account_info_iter, program_id)?;
let payer_info = next_account_info(account_info_iter)?;
let mint_info = next_account_info(account_info_iter)?;
let wrapped_meta_info = next_account_info(account_info_iter)?;
@ -1158,7 +1171,7 @@ impl Bridge {
&Self::derive_custody_seeds(bridge, mint),
subsidizer,
)?;
info!(token_program.to_string().as_str());
msg!(token_program.to_string().as_str());
let ix = spl_token::instruction::initialize_account(
token_program,
account,
@ -1235,13 +1248,13 @@ impl Bridge {
seeds: &Vec<Vec<u8>>,
subsidizer: Option<&AccountInfo>,
) -> Result<Vec<Vec<u8>>, ProgramError> {
info!("deriving key");
msg!("deriving key");
let (expected_key, full_seeds) = Bridge::derive_key(program_id, seeds)?;
if expected_key != *new_account {
return Err(Error::InvalidDerivedAccount.into());
}
info!("deploying contract");
msg!("deploying contract");
Self::create_account_raw::<T>(
program_id,
accounts,
@ -1290,4 +1303,16 @@ impl Bridge {
let s: Vec<_> = seeds.iter().map(|item| item.as_slice()).collect();
invoke_signed(&ix, accounts, &[s.as_slice()])
}
/// Get the next account info from the iterator and check that it has the given owner
pub fn next_account_info_with_owner<'a, 'b, I: Iterator<Item=&'a AccountInfo<'b>>>(
iter: &mut I,
owner: &Pubkey,
) -> Result<I::Item, ProgramError> {
let acc = iter.next().ok_or(ProgramError::NotEnoughAccountKeys)?;
if acc.owner != owner {
return Err(Error::InvalidOwner.into());
}
Ok(acc)
}
}