QuickSwapMultiPositionLiquidityManager Manual Review Findings

QSM-01M: Non-Uniform Restriction of Tick Ranges

Type	Severity	Location
Input Sanitization		QuickSwapMultiPositionLiquidityManager.sol:L274

Description:

The QuickSwapMultiPositionLiquidityManager::_setBins function will ensure that the lowerTick of each position is greater-than-or-equal-to the previous one whilst the upperTick is strictly greater-than the previous one.

Example:

Copy
273if (i >= 1) {
274    if (_positions.lowerTick[i - 1] > _positions.lowerTick[i]) {
275        revert();
276    } else {
277        require(
278            _positions.upperTick[i - 1] < _positions.upperTick[i]
279        );
280    }
281}

Recommendation:

We advise the code to ensure that both the lowerTick and upperTick values are strictly greater-than the previous position, ensuring consistency in the tick ranges and preventing the same lowerTick from being specified.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

The code was updated per our recommendation, applying the same restriction on both the lowerTick and upperTick entries ensuring a strict increase.

QSM-02M: Insecure Negation of Swap Amount

Type	Severity	Location
Mathematical Operations		QuickSwapMultiPositionLiquidityManager.sol:L124

Description:

The swapAmount variable decoded within QuickSwapMultiPositionLiquidityManager::tend is represented by an int256 variable and is negated directly using the - operator unsafely.

This is due to the fact that the int256 value space contains one extra value in the negative range, causing a value of type(int256).min to be improperly negated.

Impact:

An overflow in this case would cause a swap of 0 to be performed which would cause the ensuing QuickSwapMultiPositionLiquidityManager::_setBins call to misbehave as it would validate minimum balance-at-rest values when no swap was performed.

Example:

Copy
94(int256 swapAmount, uint160 sqrtPriceLimitX96) = abi.decode(
95    timeSensitiveData,
96    (int256, uint160)
97);
98
99// Get current pool state
100(uint160 sqrtPriceX96, int24 currentTick, , , , , ) = pool
101    .globalState();
102
103// currentTick must be close enough to TWAP tick to avoid MEV exploit
104// This is essentially a way to prevent a flashloan attack
105// even if sqrtPriceLimit is set incorrectly.
106_checkVolatility(currentTick);
107
108// Withdraw liquidity from Uniswap pool by passing in 1 and 1
109// (indicating we're withdrawing 100% of liquidity)
110_burnAndCollect(1, 1);
111
112// Update positions if desired. If newPositions is empty,
113// we'll just continue with the old positions instead.
114if (newPositions.lowerTick.length > 0) {
115    positions = newPositions;
116}
117
118// Perform a swap if desired.
119if (swapAmount != 0) {
120    bool zeroForOne = swapAmount > 0;
121    pool.swap(
122        address(this),
123        zeroForOne,
124        zeroForOne ? swapAmount : -swapAmount,
125        sqrtPriceLimitX96,
126        ""
127    );

Recommendation:

We advise the code to either decode an int248 variable akin to QuickSwapSinglePositionLiquidityManager::tend or to perform appropriate overflow checks, either of which we consider an adequate remediation to this exhibit.

Alleviation (0c3f85c7c1):

A require check was introduced that ensures the swapAmount negated is less-than the type(int256).max value, however, this check is incorrect as it will always yield true even after underflowing.

The correct require check would ensure that swapAmount is not equal to type(int256).min, in which case it would underflow.

Alleviation (b1b5eabd4d):

The newly introduced require check has been corrected to prevent the edge case of swapAmount being equal to type(int256).min, thus alleviating this exhibit in full.

QSM-03M: Inexistent Sanitization of Bin Configuration

Type	Severity	Location
Input Sanitization		QuickSwapMultiPositionLiquidityManager.sol:L143, L145, L315-L319, L322-L326

Description:

The QuickSwapMultiPositionLiquidityManager::tend function will utilize the totalWeight input argument to calculate the proportion of the contract's available balance to deposit to the QuickSwap pool, however, the proportion is not restricted to be at most equal to 100% (1e4).

As such, it is possible for a QuickSwapMultiPositionLiquidityManager::tend call to deposit more than the available balance of the contract, depositing pending fees which would cause all QuickSwapBaseLiquidityManager::_getBalance0 and QuickSwapBaseLiquidityManager::_getBalance1 function invocations to fail.

Impact:

It is possible for the vault to deposit more than its actual available balance as the weight of the QuickSwapMultiPositionLiquidityManager::tend call is not sanitized.

Example:

Copy
133uint256 balance0 = _getBalance0();
134uint256 balance1 = _getBalance1();
135
136emit Snapshot(sqrtPriceX96, balance0, balance1, totalSupply());
137
138// Create new positions in Uniswap
139if (totalWeight > 0) {
140    _setBins(
141        sqrtPriceX96,
142        // balance0 adjusted by totalWeight
143        FullMath.mulDiv(balance0, totalWeight, 1e4),
144        // balance1 adjusted by totalWeight
145        FullMath.mulDiv(balance1, totalWeight, 1e4),
146        swapAmount
147    );
148}

Recommendation:

We advise the code to ensure that the totalWeight is at most equal to 1e4 as otherwise, underflow errors would occur in the QuickSwapBaseLiquidityManager::_getBalance0 and QuickSwapBaseLiquidityManager::_getBalance1 functions.

Alleviation (0c3f85c7c11805ac412fe291f5681bef26da7244):

A require check was introduced ensuring that the totalWeight specified in an QuickSwapMultiPositionLiquidityManager::tend call is at most 100_00 thus alleviating this exhibit.