Gorples Farm - Entangle Labs

In the Farm program, the Cargo.toml file has overflow-checks = true enabled, which is a commendable practice. However, it is also recommended to incorporate safe arithmetic operations throughout the program code to further enhance security and reliability.

The provided code excerpts (non-exhaustive) reveal several occurrences of unsafe arithmetic operations.

- src/instructions/boost.rs

pub fn handle_boost(ctx: Context<Boost>, xborpa_amount: u64) -> Result<()> {
    ctx.accounts
        .harvest
        .handle_harvest(ctx.bumps.harvest.authority)?;
    require_gte!(
        ctx.accounts.harvest.virtual_balances.xborpa,
        xborpa_amount,
        CustomError::InsufficientFunds
    );
    // Substract old user position from pool
    let lp_amount = ctx.accounts.harvest.position.lp_token_amount;
    let current_multiplier = ctx.accounts.harvest.position.multiplier;
    let current_total_position = lp_amount + current_multiplier * lp_amount / 10000;
    ctx.accounts.harvest.pool.lp_supply_with_multiplier -= current_total_position;
    // Get new multiplier
    ctx.accounts.harvest.virtual_balances.xborpa -= xborpa_amount;
    ctx.accounts.harvest.pool.total_xborpa_allocated += xborpa_amount;
    ctx.accounts.harvest.position.xborpa_allocated += xborpa_amount;
    ctx.accounts.harvest.virtual_balances.total_xborpa_allocated += xborpa_amount;
    let new_multiplier = get_multiplier(
        MAX_BOOST,
        lp_amount,
        ctx.accounts.harvest.pool.lp_supply,
        ctx.accounts.harvest.position.xborpa_allocated as u128,
        ctx.accounts.harvest.pool.total_xborpa_allocated as u128,
    );
    ctx.accounts.harvest.position.multiplier = new_multiplier;
    // Update pool and user position
    let new_total_position = lp_amount + new_multiplier * lp_amount / 10000;
    ctx.accounts.harvest.pool.lp_supply_with_multiplier += new_total_position;
    Ok(())
}

In the claim_redeem instruction of the Farm program, the value of the variable to_mint is obtained by converting the value of an I80F48 type to a value of anu64 type.

Converting amount of type I80F48 to u64 can lead to precision loss, especially if amount has fractional components that will be truncated.

- src/instructions/claim_redeem.rs

pub fn handle_claim_redeem(ctx: Context<ClaimRedeem>, amount: u64) -> Result<()> {
    require_gte!(
        ctx.accounts.redeem.amount,
        amount,
        CustomError::InsufficientFunds
    );
    ctx.accounts.redeem.amount -= amount;
    let farm_config = ctx.accounts.farm_state.get_config();
    let mut amount = I80F48::from(amount - amount * farm_config.xborpa_redeem_fee / 10000);
    let cooldown = I80F48::from(farm_config.xborpa_cooldown);
    let redeem_started = ctx.accounts.redeem.started;
    let now = ctx.accounts.clock.unix_timestamp as u64;
    let elapsed = I80F48::from(
        now.checked_sub(redeem_started)
            .expect("time went backwards"),
    );
    let cooldown_ratio = I80F48::ONE - (elapsed / cooldown).clamp(I80F48::ZERO, I80F48::ONE);
    let haircut = cooldown_ratio * I80F48::from(farm_config.xborpa_haircut) / I80F48::from(10000);
    amount *= I80F48::ONE - haircut;
}

- src/instructions/harvest.rs

impl<'info> Harvest<'info> {
    pub fn handle_harvest(&mut self, authority_bump: u8) -> Result<()> {
        if !self.farm_state.is_started(self.clock.unix_timestamp as u64) {
            return Err(CustomError::NotYetStarted.into());
        }
        if self.position.last_claim_timestamp == 0 {
            self.position.last_claim_timestamp = self.clock.unix_timestamp as u64;
            return Ok(());
        }
        let raw_lp_position = self.position.lp_token_amount;
        let multiplier = self.position.multiplier;
        let total_position = raw_lp_position + raw_lp_position * multiplier / 10000;
        let last_claim = I80F48::from(self.position.last_claim_timestamp);
        self.position.last_claim_timestamp = self.clock.unix_timestamp as u64; // Update last claim
        let now = I80F48::from(self.clock.unix_timestamp);
        if self.pool.lp_supply_with_multiplier == 0 || total_position == 0 || now <= last_claim {
            return Ok(());
        }
        let elapsed_days = (now - last_claim) / I80F48::from(DAY);
        let elapsed_days_u64 = elapsed_days.to_u64().ok_or(CustomError::Overflow)?;
        let current_day = self
            .farm_state
            .get_current_day(self.clock.unix_timestamp as u64);
        /*solana_program::log::sol_log(&format!(
            "elapsed_days: {}, current_day: {}",
            elapsed_days_u64, current_day
        ));*/
        let mut user_rewards = 0;
        for day in 0..elapsed_days_u64 {
            // / 1_000_000_000_000 to adjust for formula taken from solidity with 18 decimals
            let emission_for_day = self
                .farm_state
                .get_emission_per_day((current_day - day) as u128)
                / 1_000_000_000_000;
            /*solana_program::log::sol_log(&format!(
                "emission_for_day: {} - {}",
                emission_for_day, day
            ));*/
            let pool_reward_for_day =
                emission_for_day * (self.pool.total_reward_share as u128) / 10000;
            let user_reward_for_day =
                pool_reward_for_day * total_position / self.pool.lp_supply_with_multiplier;
            user_rewards += user_reward_for_day;
        }
        if user_rewards == 0 {
            return Ok(());
        }
        let user_rewards: u64 = user_rewards.try_into().map_err(|_| CustomError::Overflow)?;
        let xborpa = user_rewards * self.pool.xborpa_percent / 10000;
        let borpa = user_rewards - xborpa;
        self.virtual_balances.xborpa += xborpa;
        self.farm_state.total_xborpa += xborpa;

It is recommended to use checked wrappers like checked_add, checked_sub, checked_div and checked_mul when performing arithmetic operations.

To avoid rounding issues, it is recommended to round down to the first valid integer, considering token decimals, when converting from I80F48 types to u64 types.

Remediation Plan

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

The current implementation of the set_admin instruction utilizes a one-step procedure for authority delegation, which in this case is translated to setting the farm.admin in FarmState account to another address. This approach raises a security concern, as it lacks a safeguard against accidental or unintended delegations to unauthorized or malicious addresses.

- src/instructions/set_admin.rs

#[derive(Accounts)]
pub struct SetAdmin<'info> {
    /// Deployer address
    #[account(signer, address = DEPLOYER.parse().expect("Deployer key not set"))]
    pub deployer: Signer<'info>,

    /// Farm state
    #[account(mut, seeds = [FARM_ROOT, b"STATE"], bump)]
    pub farm_state: Box<Account<'info, FarmState>>,
}

pub fn handle_set_admin(ctx: Context<SetAdmin>, admin: Pubkey) -> Result<()> {
    ctx.accounts.farm_state.set_admin(admin);
    Ok(())
}

- src/state.rs

    pub fn set_admin(&mut self, admin: Pubkey) {
        self.admin = admin;
    }

To mitigate this risk, it is recommended to enforce a multi-step admin transfer mechanism. In this approach, the current admin initiates the transfer by setting a specific address as pending_admin. Subsequently, the pending_admin accepts the transfer, completing the delegation process.

Remediation Plan

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

It was identifying during the assessment of the program Farm in-scope that its dependencies for the Anchor framework and also for Solana are not current.

[[package]]
name = "anchor-lang"
version = "0.29.0"

{ ... }

[[package]]
name = "solana-program"
version = "1.16.27"

It is recommended to update dependencies to their current versions, as specified:

• Solana: v1.18.15

• Anchor: v0.30.0

Remediation Plan

ACKNOWLEDGED: The Entangle team acknowledged this finding.

The provided Farm program in-scope performs some administrative tasks such as configure_farm, configure_pool and add_pool. These instructions are closely related to state modifications that will affect the FarmState account.

In the Configure Pool instruction, there is no validation for the parameters total_reward_share, deposit_fee_rate and xborpa_percent, as follows:

- src/instructions/configure_pool.rs

pub fn handle_configure_pool(
    ctx: Context<ConfigurePool>,
    total_reward_share: u64,
    deposit_fee_rate: u64,
    xborpa_percent: u64,
) -> Result<()> {
    ctx.accounts
        .farm_state
        .register_pool_allocation(ctx.accounts.pool.total_reward_share, total_reward_share)?;
    ctx.accounts.pool.total_reward_share = total_reward_share;
    ctx.accounts.pool.deposit_fee_rate = deposit_fee_rate;
    ctx.accounts.pool.xborpa_percent = xborpa_percent;
    Ok(())
}

In the Configure Farm instruction, there is no validation in place for the xborpa_cooldown, xborpa_redeem_fee and xborpa_haircut parameters, as follows:

pub fn handle_configure_farm(
    ctx: Context<ConfigureFarm>,
    xborpa_cooldown: u64,
    xborpa_redeem_fee: u64,
    xborpa_haircut: u64,
) -> Result<()> {
    ctx.accounts
        .farm_state
        .set_params(xborpa_cooldown, xborpa_redeem_fee, xborpa_haircut);
    Ok(())
}

In the Create Pool instruction, there is no validation in place for the parameters total_reward_share, deposit_fee_rate and xborpa_percent, as follows:

- src/instructions/create_pool.rs

pub fn handle_create_pool(
    ctx: Context<CreatePool>,
    total_reward_share: u64,
    deposit_fee_rate: u64,
    xborpa_percent: u64,
) -> Result<()> {
    ctx.accounts.pool.mint0 = ctx.accounts.mint0.key();
    ctx.accounts.pool.mint1 = ctx.accounts.mint1.key();
    ctx.accounts
        .farm_state
        .register_pool_allocation(0, total_reward_share)?;
    ctx.accounts.pool.total_reward_share = total_reward_share;
    ctx.accounts.pool.deposit_fee_rate = deposit_fee_rate;
    ctx.accounts.pool.xborpa_percent = xborpa_percent;
    Ok(())
}

All the values casted in these examples are used widely in the protocol, in instructions like harvest, deposit, withdraw, claim_redeem, and others. The lack of validation of the inputs given to configuration instructions can lead to miscalculations when executing user-facing instructions.

It is recommended to perform input validation over the parameters that are given to administrative instructions.

Remediation Plan

ACKNOWLEDGED: The Entangle team acknowledged this finding.