sherlock-audit / 2024-09-orderly-network-solana-contract-judging

0 stars 0 forks source link

Dizzy Green Mantis - Missing access control on `oapp_lz_receive::apply` #142

Open sherlock-admin3 opened 4 days ago

sherlock-admin3 commented 4 days ago

Dizzy Green Mantis

High

Missing access control on oapp_lz_receive::apply

Summary

oapp_lz_receive::apply is the entrypoint for LZ to interact with Orderly Solana programs, and there is no access control on who can call such method. This can lead to unauthorized message being parsed and executed.

Root Cause

lib::lz_receive lists open methods of the vault program, to interact with LZ, lz_receive needs to be called in order to receive messages. When lz_receive is called, oapp_lz_receive::apply is executed:

pub struct OAppLzReceive<'info> {
    #[account(mut)]
    pub payer: Signer<'info>,
    #[account(
        seeds = [
            PEER_SEED,
            &oapp_config.key().to_bytes(),
            &params.src_eid.to_be_bytes()
        ],
        bump = peer.bump,
        constraint = peer.address == params.sender @OAppError::InvalidSender
    )]
    pub peer: Account<'info, Peer>,
    #[account(
        seeds = [OAPP_SEED],
        bump = oapp_config.bump
    )]
    pub oapp_config: Account<'info, OAppConfig>,
    /// CHECK
    #[account()]
    pub user: AccountInfo<'info>,

    #[account(
        mut,
        associated_token::mint = deposit_token,
        associated_token::authority = user
    )]
    pub user_deposit_wallet: Account<'info, TokenAccount>,

    #[account(
        mut,
        seeds = [VAULT_AUTHORITY_SEED],
        bump = vault_authority.bump,
    )]
    pub vault_authority: Account<'info, VaultAuthority>,

    #[account(
        mut,
        associated_token::mint = deposit_token,
        associated_token::authority = vault_authority
    )]
    pub vault_deposit_wallet: Account<'info, TokenAccount>,

    #[account()]
    pub deposit_token: Account<'info, Mint>,

    pub token_program: Program<'info, Token>,

    pub system_program: Program<'info, System>,
}

impl<'info> OAppLzReceive<'info> {
    fn transfer_token_ctx(&self) -> CpiContext<'_, '_, '_, 'info, Transfer<'info>> {
        let cpi_accounts = Transfer {
            from: self.vault_deposit_wallet.to_account_info(),
            to: self.user_deposit_wallet.to_account_info(),
            authority: self.vault_authority.to_account_info(),
        };
        let cpi_program = self.token_program.to_account_info();
        CpiContext::new(cpi_program, cpi_accounts)
    }

    pub fn apply(ctx: &mut Context<OAppLzReceive>, params: &OAppLzReceiveParams) -> Result<()> {
        let seeds: &[&[u8]] = &[OAPP_SEED, &[ctx.accounts.oapp_config.bump]];

        let accounts_for_clear = &ctx.remaining_accounts[0..Clear::MIN_ACCOUNTS_LEN];
        let _ = oapp::endpoint_cpi::clear(
            ctx.accounts.oapp_config.endpoint_program,
            ctx.accounts.oapp_config.key(),
            accounts_for_clear,
            seeds,
            ClearParams {
                receiver: ctx.accounts.oapp_config.key(),
                src_eid: params.src_eid,
                sender: params.sender,
                nonce: params.nonce,
                guid: params.guid,
                message: params.message.clone(),
            },
        )?;

        if ctx.accounts.vault_authority.order_delivery {
            require!(
                params.nonce == ctx.accounts.vault_authority.inbound_nonce + 1,
                OAppError::InvalidInboundNonce
            );
        }

        ctx.accounts.vault_authority.inbound_nonce = params.nonce;

        // msg!(
        //     "nonce received: {:?}",
        //     ctx.accounts.vault_authority.inbound_nonce
        // );

        let lz_message = LzMessage::decode(&params.message).unwrap();
        msg!("msg_type: {:?}", lz_message.msg_type);
        if lz_message.msg_type == MsgType::Withdraw as u8 {
            let withdraw_params = AccountWithdrawSol::decode_packed(&lz_message.payload).unwrap();

            let vault_authority_seeds =
                &[VAULT_AUTHORITY_SEED, &[ctx.accounts.vault_authority.bump]];

            // msg!("Withdraw amount = {}", withdraw_params.token_amount);
            let amount_to_transfer = withdraw_params.token_amount - withdraw_params.fee;
            transfer(
                ctx.accounts
                    .transfer_token_ctx()
                    .with_signer(&[&vault_authority_seeds[..]]),
                amount_to_transfer,
            )?;

            let vault_withdraw_params: VaultWithdrawParams = withdraw_params.into();
            emit!(Into::<VaultWithdrawn>::into(vault_withdraw_params.clone()));
        } else {
            msg!("Invalid message type: {:?}", lz_message.msg_type);
        }

        Ok(())
    }
}

But in this program there is no has_one to restrict who can call this method. This will allow non-LZ endpoint addresses to call this method, when a malicious message is crafted and provided, it can cause unauthorized withdraw and even drain of the vault, as transfer action does not check balance.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Unauthorized call of lz_receive, and can lead to drain of vault.

PoC

No response

Mitigation

Set oapp_lz_receive's admin to be LZ endpoint, and add has_one to ensure access control.