LZ_security - The OAppLzReceive contract lacks underflow checks, which could allow an attacker to exploit this vulnerability to steal funds.

[sherlock-admin2]

LZ_security

High

The OAppLzReceive contract lacks underflow checks, which could allow an attacker to exploit this vulnerability to steal funds.

Summary

The OAppLzReceive contract lacks underflow checks, which could allow an attacker to exploit this vulnerability to steal funds.

Root Cause

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/main/solana-vault/packages/solana/contracts/programs/solana-vault/src/instructions/oapp_instr/oapp_lz_receive.rs#L117

 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(())
    }
}

Since there is no check on the size of withdraw_params.token_amount and withdraw_params.fee, when token_amount < fee, the calculated amount_to_transfer could result in a very large number. For example:

let token_amount: u64 = 5;
let fee: u64 = 10;
let amount_to_transfer = token_amount - fee; // Underflow occurs
// amount_to_transfer = 18446744073709551611

As a result, an attacker could exploit this vulnerability to steal funds from the vault.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

attacker set withdraw_params.token_amount less than withdraw_params.fee

Impact

The protocol suffers a loss of funds.

PoC

No response

Mitigation

// Using checked_sub method
let amount_to_transfer = withdraw_params
    .token_amount
    .checked_sub(withdraw_params.fee)
    .ok_or(VaultError::InsufficientTokenAmount)?;