Gorples Chef & Farm updates - Entangle Labs

The AddLiquidity instruction, called by deposit, calculates the corresponding amounts to increase liquidity based on the user-provided amount_0 and amount_1, considering the following:

• Transfer fees that will be internally deducted during transfer_checked.

• fee0 and fee1, based on the deposit_fee_rate of the pool, which will be charged for adding liquidity

Once calculated, the liquidity is increased accordingly. If there is an excess amount between what was transferred to token_account_0 and token_account_1 and the amount used to increase liquidity after deducting the mentioned fees, this excess is refunded to the user as refund0 and refund1.

If either of the calculated refund values (refund0 or refund1) is greater than zero but less than the transfer fee deducted during TransferChecked, it will panic due to an overflow error. This is due to a lack of verification in the instruction handler, as the transfer fee will be subtracted from the intended refund amount.

add_liquidity.rs

  ctx.accounts.token_account_0.reload()?;
    ctx.accounts.token_account_1.reload()?;
    let vault0_after_deposit = ctx.accounts.token_account_0.amount;
    let vault1_after_deposit = ctx.accounts.token_account_1.amount;
    let leftover0 = vault0_after_deposit - vault0_before_transfer;
    let leftover1 = vault1_after_deposit - vault1_before_transfer;
    solana_program::log::sol_log(&format!(
        "leftover0: {} leftover1: {}",
        leftover0, leftover1
    ));
    let refund0 = if leftover0 > fee0 {
        leftover0 - fee0
    } else {
        0
    };
    let refund1 = if leftover1 > fee1 {
        leftover1 - fee1
    } else {
        0
    };
    solana_program::log::sol_log(&format!("refund0: {} refund1: {}", refund0, refund1));
    if refund0 > 0 {
        // Transfer token 0
        let accounts = TransferChecked {
            from: ctx.accounts.token_account_0.to_account_info(),
            mint: ctx.accounts.token_mint_0.to_account_info(),
            to: ctx.accounts.token_source_0.to_account_info(),
            authority: ctx.accounts.authority.to_account_info(),
        };
        let cpi = CpiContext::new_with_signer(ctx.accounts.get_token_program_0(), accounts, seeds);
        transfer_checked(cpi, refund0, ctx.accounts.token_mint_0.decimals)?;
    }
    if refund1 > 0 {
        // Transfer token 1
        let accounts = TransferChecked {
            from: ctx.accounts.token_account_1.to_account_info(),
            mint: ctx.accounts.token_mint_1.to_account_info(),
            to: ctx.accounts.token_source_1.to_account_info(),
            authority: ctx.accounts.authority.to_account_info(),
        };
        let cpi = CpiContext::new_with_signer(ctx.accounts.get_token_program_1(), accounts, seeds);
        transfer_checked(cpi, refund1, ctx.accounts.token_mint_1.decimals)?;
    }

To address this issue, it is recommended to add to the validation a check that verifies the amount to be transferred as refund is greater than zero and is also greater than the corresponding transfer fee, if it is any.

Remediation Plan

RISK ACCEPTED: The Entangle team accepted the risk related to this finding.

During the initialization of the FarmState, the start_time is assigned a default value of zero. If this value is not modified before users make deposits, it will create the impression that the FarmState has started, allowing deposit to be performed and rewards to be obtained.

harvest.rs

impl FarmState {
    pub const LEN: usize = 8 + 32 + 8 * 7;

    pub fn new(config: FarmConfig) -> Self {
        Self {
            admin: config.admin,
            xborpa_cooldown: config.xborpa_cooldown,
            xborpa_redeem_fee: config.xborpa_redeem_fee,
            xborpa_haircut: config.xborpa_haircut,
            total_borpa_minted: 0,
            total_xborpa: 0,
            allocated_reward_share: 0,
            start_time: 0,
        }
    }

When users make their first deposits, their new positions will be initialized through the Harvest instruction, updating their last_claim_timestamp to the current unix_timestamp. The Harvest instruction handles the necessary calculations for users to receive rewards based on the last_claim_timestamp of their position at the time of the call.

However, the SetStartRewards instruction allows the admin to modify the start_time at any moment.

set_start_rewards.rs

pub fn handle_set_start_rewards(ctx: Context<SetStartRewards>, start_time: u64) -> Result<()> {
    ctx.accounts
        .farm_state
        .set_start_rewards(start_time, ctx.accounts.clock.unix_timestamp as u64)?;
    Ok(())

state.rs

pub fn set_start_rewards(
        &mut self,
        start_time: u64,
        now: u64,
    ) -> std::result::Result<(), CustomError> {
        /*if self.start_time != 0 && now > self.start_time {
            return Err(CustomError::RewardsAlreadyStarted);
        }*/
        if now > start_time && start_time != 0 {
            return Err(CustomError::InvalidStartTime);
        }
        self.start_time = start_time;
        Ok(())

If this value is changed to postpone the start_time after deposits have been made, users who have previously deposited will lose the rewards corresponding to the days that have passed since they deposited.This occurs because rewards cannot be calculated until the Farm has started. Once the Farm begins, the last_claim_timestamp will be updated based on the time of the call.

harvest.rs

  if !self.farm_state.is_started(self.clock.unix_timestamp as u64) {
            return Ok(());
        }

        if self.position.last_claim_timestamp == 0
            || self.position.last_claim_timestamp < self.farm_state.start_time
        {
            self.position.last_claim_timestamp = self.clock.unix_timestamp as u64;
            return Ok(());
        }

To address this issue, consider implementing one of the following options:

• Disallow deposits if the Farm has is not started.

• Prevent changes to the start time of the Farm State once deposits have been made.

Remediation Plan

RISK ACCEPTED: The Entangle team accepted the risk related to this finding.

The Harvest instruction requires several accounts to be provided, the borpa_target_vault among them, where the amount of borpa rewards will be minted to. However, it is not validated that the authority of the account matches that of the signing user, allowing any borpa token account to be provided.

harvest.rs

    /// Target vault to claim $BORPA to
    #[account(mut)]
    pub borpa_target_vault: Box<InterfaceAccount<'info, TokenAccount>>,

let accounts = MintBorpa {
            mint_authority: self.authority.to_account_info(),
            authority: self.borpa_core_authority.to_account_info(),
            config: self.borpa_core_config.to_account_info(),
            mint: self.borpa_mint.to_account_info(),
            vault: self.borpa_target_vault.to_account_info(),
            token_program: self.token_program_2022.to_account_info(),
        };
        let bump = &[authority_bump][..];
        let seed = &[FARM_ROOT, b"AUTHORITY", bump][..];
        let seeds = &[seed];
        let cpi = CpiContext::new_with_signer(self.borpa_core.to_account_info(), accounts, seeds);
        self.farm_state.register_mint(borpa)?;
        mint_borpa(cpi, borpa)?;

Consider adding the missing validation as a best practice to ensure that borpa rewards are minted in the user's borpa token account if expected.

Remediation Plan

ACKNOWLEDGED: The Entangle team has acknowledged this finding.