SinglePositionLiquidityManager Manual Review Findings

SPL-01M: Unprotected Inversion of Swap Amount

Type	Severity	Location
Mathematical Operations		SinglePositionLiquidityManager.sol:L81

Description:

The linked inversion of swapAmount (-swapAmount) is prone to a hidden overflow. The int256 variable represents the range of [-2**255, 2**255 - 1], meaning that if the swapAmount is defined as type(int256).min the value will underflow in that instruction and cause an incorrect swap amount to be passed in the swap call.

Example:

Copy
48function tend(
49    uint256 totalWeight,
50    int24 newLowerTick,
51    int24 newUpperTick,
52    int256 swapAmount,
53    uint160 sqrtPriceLimitX96
54) external onlyRole(ORCHESTRATOR_ROLE) whenNotPaused {
55    // Get current pool state
56    (uint160 sqrtPriceX96, int24 currentTick, , , , , ) = pool.slot0();
57
58    // currentTick must be close enough to TWAP tick to avoid MEV exploit
59    // This is essentially a way to prevent a flashloan attack
60    // even if sqrtPriceLimit is set incorrectly.
61    _checkVolatility(currentTick);
62
63    // Withdraw liquidity from Uniswap pool by passing in 1 and 1
64    // (indicating we're withdrawing 100% of liquidity)
65    _burnAndCollect(1, 1);
66
67    // Update positions if desired.
68    // If both are set to zero (or just the same amount) then we won't update the positions,
69    // instead leaving them as they were.
70    if (newLowerTick != newUpperTick) {
71        lowerTick = newLowerTick;
72        upperTick = newUpperTick;
73    }
74
75    // Perform a swap if desired.
76    if (swapAmount > 0) {
77        bool zeroForOne = swapAmount > 0;
78        pool.swap(
79            address(this),
80            zeroForOne,
81            zeroForOne ? swapAmount : -swapAmount,
82            sqrtPriceLimitX96,
83            ""
84        );

Recommendation:

While not an active security threat, it is a misbehaviour present in the contract that should be avoided by disallowing an abnormal swapAmount value or by reducing the input variable's accuracy from int256 to int248 and upcasting within the function, ensuring the inversion will never overflow.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

The code was updated to perform the latter of our two recommendations; reducing the input accuracy of swapAmount from uint256 to uint248 and upcasting within it. As a result, we consider this exhibit alleviated in full.

SPL-02M: Inverse Check Application

Type	Severity	Location
Mathematical Operations		SinglePositionLiquidityManager.sol:L229, L233

Description:

The linked check is meant to validate that at least 95% of the token has been deposited, however, it presently validates that at least 5% of the token has been deposited as it evaluates whether the current post-deposit funds of the contract are less than 95% of the expected to deposit amount.

Impact:

The arbitrage protection imposed by the system is currently insufficient and possible to circumvent.

Example:

Copy
226if (swapAmount > 0) {
227    // Require that at least 95% of t1 has been deposited
228    // (ensuring that swap amount wasn't too great)
229    require(_getBalance1() < (t1ToDeposit * 95) / 100, "swap");
230} else if (swapAmount < 0) {
231    // Require that at least 95% of t0 has been deposited
232    // (ensuring that swap amount wasn't too great)
233    require(_getBalance0() < (t0ToDeposit * 95) / 100, "swap");
234}

Recommendation:

We advise the checks to be corrected by adjusting the 95 literal to 5, adequately representing 5% instead of 95%.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

Both checks have been corrected to evaluate that the remaining balance is less than 5% of the original to-deposit funds, greatly enhancing the function's resilience against arbitrage attacks.