Vast Umber Walrus - Refund logic in `Locker::initializeCollection()` incorrectly handles the return of unused tokens.

[sherlock-admin3]

Vast Umber Walrus

Medium

Refund logic in Locker::initializeCollection() incorrectly handles the return of unused tokens.

Summary

The refund logic in the Locker::initializeCollection() fails to work as intended. nativeTokens are transferred from the user to the implementation contract, but no tokens are transferred to the Locker contract.

As a result, the process designed to refund unused native tokens to the user during the creation of initial liquidity does not execute correctly, leading to no refunds being made.

Vulnerability Detail

When the Locker::initializeCollection() function is called, it attempts to refund any unused native tokens back to the user. However, the logic does not work as intended because the tokens are transferred from the user to the implementation contract, and there is no token transfer to the Locker contract.

Locker::initializeCollection()

File: Locker.sol
367:     function initializeCollection(address _collection, uint _eth, uint[] calldata _tokenIds, uint _tokenSlippage, uint160 _sqrtPriceX96) public virtual whenNotPaused collectionExists(_collection) {
---
379:         // Convert the tokens into ERC20's which will return at a rate of 1:1
380:         deposit(_collection, _tokenIds, address(_implementation));
---
382:         // Send the native ETH equivalent token into the implementation
383:         uint startBalance = nativeToken.balanceOf(address(this));
384:@>       nativeToken.transferFrom(msg.sender, address(_implementation), _eth);
---
388:         _implementation.initializeCollection(_collection, _eth, tokens, _tokenSlippage, _sqrtPriceX96);
---
394:         // Refund any unused relative token to the user
395:@>       nativeToken.transfer(
396:             msg.sender,
397:             startBalance - nativeToken.balanceOf(address(this))
398:         );
399:     }

As a result, no refunds are made to the user. The logic assumes that if there are unused tokens after liquidity is provided, those tokens should be refunded, but this process fails.

Furthermore, this issue does not aim to address user mistakes in specifying excessive native tokens beyond the required price. Instead, the issue lies in the logic designed to refund unused tokens that were intended to provide liquidity at the specified price.

Impact

The logic does not work as intended, and users who provide liquidity might end up losing their unused tokens, as the system fails to refund any leftover tokens.

Code Snippet

Locker::initializeCollection()

File: Locker.sol
367:     function initializeCollection(address _collection, uint _eth, uint[] calldata _tokenIds, uint _tokenSlippage, uint160 _sqrtPriceX96) public virtual whenNotPaused collectionExists(_collection) {
---
379:         // Convert the tokens into ERC20's which will return at a rate of 1:1
380:         deposit(_collection, _tokenIds, address(_implementation));
---
382:         // Send the native ETH equivalent token into the implementation
383:         uint startBalance = nativeToken.balanceOf(address(this));
384:@>       nativeToken.transferFrom(msg.sender, address(_implementation), _eth);
---
388:         _implementation.initializeCollection(_collection, _eth, tokens, _tokenSlippage, _sqrtPriceX96);
---
394:         // Refund any unused relative token to the user
395:@>       nativeToken.transfer(
396:             msg.sender,
397:             startBalance - nativeToken.balanceOf(address(this))
398:         );
399:     }

Tool used

Manual Review

Recommendation

In the context of the contest, the implementation contract refers to the UniswapImplementation contract. Therefore, the recommendations will be based on this code.

File: Locker.sol
function initializeCollection(address _collection, uint _eth, uint[] calldata _tokenIds, uint _tokenSlippage, uint160 _sqrtPriceX96) public virtual whenNotPaused collectionExists(_collection) {

---
     // Send the native ETH equivalent token into the implementation
     uint startBalance = nativeToken.balanceOf(address(this));
     nativeToken.transferFrom(msg.sender, address(_implementation), _eth);

---

    _implementation.initializeCollection(_collection, _eth, tokens, _tokenSlippage, _sqrtPriceX96);

---
    // Refund any unused relative token to the user
    nativeToken.transfer(
        msg.sender,
-       startBalance - nativeToken.balanceOf(address(this))
+       nativeToken.balanceOf(address(this)) - startBalance
    );
}

File: UniswapImplementation.sol
function initializeCollection(address _collection, uint _amount0, uint _amount1, uint _amount1Slippage, uint160 _sqrtPriceX96) public override {
    // Ensure that only our {Locker} can call initialize
    if (msg.sender != address(locker)) revert CallerIsNotLocker();
+   Currency _nativeCurrency = poolParams.currencyFlipped ? poolKey.currency1 : poolKey.currency0;
+   uint256 startNativeBalance = _nativeCurrency.balanceOfSelf();
---
    // Obtain the UV4 lock for the pool to pull in liquidity
    poolManager.unlock(
        abi.encode(CallbackData({
            poolKey: poolKey,
            liquidityDelta: LiquidityAmounts.getLiquidityForAmounts({
                sqrtPriceX96: _sqrtPriceX96,
                sqrtPriceAX96: TICK_SQRT_PRICEAX96,
                sqrtPriceBX96: TICK_SQRT_PRICEBX96,
                amount0: poolParams.currencyFlipped ? _amount1 : _amount0,
                amount1: poolParams.currencyFlipped ? _amount0 : _amount1
            }),
            liquidityTokens: _amount1,
            liquidityTokenSlippage: _amount1Slippage
        })
    ));

+    uint256 afterNativeBalance = _nativeCurrency.balanceOfSelf();
+    if ((startNativeBalance - afterNativeBalance) > 0){
+        SafeTransferLib.safeTransfer(Currency.unwrap(_currency), msg.sender, (startNativeBalance - afterNativeBalance));
    }
}