Precision Control Issue in Curve Liquidity Optimization Function

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xca94a59f2331a33b1c9e2e115706c9c153fcfd5c8699627c2004197d1905bd26
Severity: high

Description:
Description
In the CurveLiqArbitrage.sol contract, the findBestProportion function is designed to identify the optimal liquidity addition ratio. However, the function’s epsilon parameter lacks adequate boundary checks, potentially leading to:
1.Impacting overall protocol liquidity
2.Increase slippage
3. Excessive invalid iterations
4. High gas consumption
5. Precision issues in calculations

Attack Scenario
All liquidity addition operations using the findBestProportion function could impact the protocol's liquidity efficiency and increase user gas costs. If liquidity providers use suboptimal ratios, they will receive fewer LP tokens, potentially causing the pool's balance to deviate from the ideal state, which could increase slippage and affect other users' trading costs. The optimal proportion should be close to 0.8e18; deviating from this ratio reduces liquidity efficiency, and cumulative effects can cause a sustained decline in pool efficiency.

Although the single-instance impact is small (around 0.01%), the impact could be amplified under the following conditions: large liquidity deposits, high-frequency operations, or during market volatility. Therefore, while the single-instance impact is minor, the functionality’s effectiveness could be compromised over time, reducing liquidity provision efficiency, affecting pool balance, increasing transaction costs, and thereby impacting overall protocol liquidity efficiency.

Attachments

1. Proof of Concept (PoC) File
   Vulnerable Code

src/router/util/CurveLiqArbitrage.sol:findBestProportion#L66-L70

    function findBestProportion(
        address curvePool, // Curve pool address
        uint256 depositInToken0, // Total deposit amount in terms of token0
        uint256 epsilon // Error tolerance (18 decimal places)
    ) public view returns (uint256) {

• Parameter Control Issue

  • epsilon is a public parameter and can be set arbitrarily.
  • Testing shows a reasonable range is between 1e2 and ``1e3, 1e3 and `1e4`.
  • However, the contract does not enforce these limits.

• Impact

  • When epsilon is set too low, the while loop will iterate excessively.
  • Each iteration requires a call to the external contract’s calc_token_amount function.
  • This can lead to failed transactions or high gas costs.

• Verification
  Test cases indicate that the optimal ratio provides about a 0.01% advantage:

test/Router/CurveLiqArbitrageTest.t.sol:testCDFindBestProportionFuzz#L2978-L3037

        epsilon = bound(epsilon, 1e2, 1e3);

        data.baseProportion1 = CurveLiqArbitrage(curveLiqArbitrage).findBestProportion(
            curvePool,
            ibtAmount,
            epsilon
        );

...

        assertGe(
            data.lpAmount1,
            data.lpAmount2.mulDiv(9999, 10000),
            "Best proportion found 2 is sub optimal 1"
        );
        assertGe(
            data.lpAmount1,
            data.lpAmount3.mulDiv(9999, 10000),
            "Best proportion found 2 is sub optimal 2"
        );

2. Revised Code File (Optional)

• Remove the epsilon Parameter: Instead, implement an internal calculation to dynamically compute precision based on pool state and trade volume. Introduce an adaptive mechanism to increase precision when needed, with a maximum iteration limit as a safeguard.

  • Refer to other projects for implementation insights:
    • Uniswap V3 uses price range-based dynamic precision
    • Curve’s other optimization functions often dynamically adjust parameters based on pool state
    • Balancer uses weight factors to dynamically adjust precision

• Curve StableSwap Documentation

• Golden Section Search Algorithm

• Alternatively, Enforce Reasonable Limits on epsilon: Consider caching the optimal ratio and providing fallback options under extreme market conditions.

[yanisepfl]

Hello,

We classified this issue as Invalid because the view functions of this contract are only meant to be used as references of how to add liquidity to Curve Pools, users are responsible for the way they decide to add liquidity. As you can see, they aren't called in our contracts and thus cannot result in theft of funds.

Thanks