Gorples SOL Chef - Entangle Labs

During the analysis of the Remove Liquidity, Add Pool, and Swap instructions, it was observed that the slippage value is hard-coded in different instances, as demonstrated:

Remove Liquidity

During the analysis of the handle_remove_liquidity function, part of the Remove Liquidity instruction, it was identified that the values for the variables amount_0_with_slippage and amount_1_with_slippage are based on calculations that are considering 1000 (10% - ten percent) as slippage value when performing the calculation through the amount_with_slippage utility function.

These values are subsequently processed through the amount_0_min and amount_1_min variables, that performs checked_sub with transfer_fee_0 and transfer_fee_1 values, and which will be ultimately used in the CPI to decrease_liquidity_v2.

- src/instructions/remove_liquidity.rs

    let amount_0_with_slippage = amount_with_slippage(amount_0, 1000, false);
    let amount_1_with_slippage = amount_with_slippage(amount_1, 1000, false);

    let amount_0_min = amount_0_with_slippage
        .checked_sub(transfer_fee_0)
        .ok_or(CustomError::Overflow)?;
    let amount_1_min = amount_1_with_slippage
        .checked_sub(transfer_fee_1)
        .ok_or(CustomError::Overflow)?;

    let cpi = CpiContext::new_with_signer(ctx.accounts.raydium.to_account_info(), accounts, seeds);
    raydium_amm_v3::cpi::decrease_liquidity_v2(cpi, liquidity, amount_0_min, amount_1_min)?;

The current calculations take into account a slippage tolerance value of 10% (1000 basis points), which is excessively high for a Remove Liquidity operation. It is important to note that with this setting, up to ten percent of the traded amounts could potentially be lost due to slippage.

Add Pool

Likewise, during the assessment of the handle_add_pool function, it is possible to observe the same behavior, as follows:

- src/instructions/add_pool.rs

    let amount_0_with_slippage = amount_with_slippage(amount_0, 1000, true);
    let amount_1_with_slippage = amount_with_slippage(amount_1, 1000, true);

    let amount_0_max = amount_0_with_slippage.checked_add(transfer_fee_0).unwrap();
    let amount_1_max = amount_1_with_slippage.checked_add(transfer_fee_1).unwrap();

    raydium_amm_v3::cpi::open_position_v2(
        cpi,
        pool_config.tick_lower_index,
        pool_config.tick_upper_index,
        pool_config.tick_array_lower_start_index,
        pool_config.tick_array_upper_start_index,
        liquidity,
        amount_0_max,
        amount_1_max,
        false,
        None,
    )?;

The aforementioned calculations take into account a slippage tolerance value of 10% (1000 basis points), which is excessively high for Add Pool and/or Remove Liquidity operations. It is important to note that with this setting, up to ten percent of the traded amounts could potentially be lost due to slippage, resulting in permanent financial loss.

Swap

In the handle_swap function, which is part of the Swap instruction, it was identified that the value for the parameter other_amount_threshold is hard-coded to 0 (zero).

- src/instructions/swap.rs

        swap_v2(cpi, swap_0_amount, 0, 0, true)?;

        swap_v2(cpi, swap_0_amount, 0, 0, true)?;

Accordingly to the swap_v2, Raydium's CLMM program documentation, other_amount_threshold dictates the slippage tolerance for the swap operation.

In the specific implementation of the handle_swap function, that performs a CPI to the swap_v2 instruction on Raydium's CLMM, where is_base_input is set to true, the expected return value is the max_amount_out.

- raydium-clmm/programs/amm/src/instructions/swap_v2.rs

pub fn swap_v2<'a, 'b, 'c: 'info, 'info>(
    ctx: Context<'a, 'b, 'c, 'info, SwapSingleV2<'info>>,
    amount: u64,
    other_amount_threshold: u64,
    sqrt_price_limit_x64: u128,
    is_base_input: bool,
) -> Result<()> {
    let amount_result = exact_internal_v2(
        ctx.accounts,
        ctx.remaining_accounts,
        amount,
        sqrt_price_limit_x64,
        is_base_input,
    )?;
    if is_base_input {
        require_gte!(
            amount_result,
            other_amount_threshold,
            ErrorCode::TooLittleOutputReceived
        );
    } else {
        require_gte!(
            other_amount_threshold,
            amount_result,
            ErrorCode::TooMuchInputPaid
        );
    }

    Ok(())
}

The practice of hard-coding the other_amount_threshold parameter to 0 (zero) could result in undesirable outcomes, such as loss-making (unprofitable) swap operations. This is because amount_result will always be greater or equal than 0, and therefore, even for a relatively small result for max_amount_out, the transaction would still go through, because of lack of slippage check.

It is strongly advised to avoid hard-coding slippage values when executing Cross-Program Invocations (CPIs), as a safe slippage threshold is essential for transactions to be successfully processed without financial damage.

For the Add Pool and Remove Liquidity instructions, it is recommended to use a safe threshold, passed as a parameter, to the slippage value, instead of using hard-coded 1000 units.

For the Swap instruction, it is recommended to add the other_amount_threshold parameter to the handle_swap function, so this parameter does not operate under hard-coded 0 value when performing the CPI to swap_v2.

Additionally, it is recommended to enforce a safe threshold for these parameters, for example, requiring a minimum/maximum value for other_amount_threshold (slippage).

Remediation Plan

RISK ACCEPTED: The Entangle team has accepted the risk related to the issue.