Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

lil.eth - Underflow in borrow() Function #140

Closed
sherlock-admin2 opened this issue Oct 23, 2023 · 0 comments
Closed

lil.eth - Underflow in borrow() Function #140

sherlock-admin2 opened this issue Oct 23, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Oct 23, 2023

lil.eth

medium

Underflow in borrow() Function

Summary

The borrow() function may end up in scenarios where cache.borrowedAmount is less than cache.holdTokenBalance, The line uint256 borrowingCollateral = cache.borrowedAmount - cache.holdTokenBalance; within the borrow() function is will revert due to underflow, leading to an UniV3 position available for Wagmi protocol which cannot be borrowed.

Vulnerability Detail

cache.borrowedAmount is the result from this function :

function _getSingleSideRoundUpBorrowedAmount(
        bool zeroForSaleToken,
        int24 tickLower,
        int24 tickUpper,
        uint128 liquidity
    ) private pure returns (uint256 borrowedAmount) {
        borrowedAmount = (
            zeroForSaleToken
                ? LiquidityAmounts.getAmount1ForLiquidity(
                    TickMath.getSqrtRatioAtTick(tickLower),
                    TickMath.getSqrtRatioAtTick(tickUpper),
                    liquidity
                )
                : LiquidityAmounts.getAmount0ForLiquidity(
                    TickMath.getSqrtRatioAtTick(tickLower),
                    TickMath.getSqrtRatioAtTick(tickUpper),
                    liquidity
                )
        );
        if (borrowedAmount > Constants.MINIMUM_BORROWED_AMOUNT) {
            ++borrowedAmount;
        } else {
            revert TooLittleBorrowedLiquidity(liquidity);
        }
    }

which basically return number of holdBalance tokens needed to re-gain liquidity amount in a single UniV3 position.

On the other way cache.holdTokenBalance is calculated by the number of holdToken extracted at current tick + swapped saleToken collected :

        (saleTokenBalance, cache.holdTokenBalance) = _getPairBalance(
            params.saleToken,
            params.holdToken
        );
        // Check if the sale token balance is greater than 0
        if (saleTokenBalance > 0) {
            if (params.externalSwap.swapTarget != address(0)) {
                // Call the external swap function and update the hold token balance in the cache
                cache.holdTokenBalance += _patchAmountsAndCallSwap(
                    params.saleToken,
                    params.holdToken,
                    params.externalSwap,
                    saleTokenBalance,
                    0
                );
            } else {
                // Call the internal v3SwapExactInput function and update the hold token balance in the cache
                cache.holdTokenBalance += _v3SwapExactInput(
                    v3SwapExactInputParams({
                        fee: params.internalSwapPoolfee,
                        tokenIn: params.saleToken,
                        tokenOut: params.holdToken,
                        amountIn: saleTokenBalance,
                        amountOutMinimum: 0
                    })
                );
            }
        }

I see 3 scenarios where cache.holdTokenBalance can be > cache.borrowedAmount and lead to a DOS :

  1. If the position has earned fees in saleToken, then those could be swapped for holdToken by either internal or external swaps, thus increasing the cache.holdTokenBalance.
  2. If the price has moved favorably since the position was established, the amount of holdToken needed for the same liquidity could be less than initially calculated.
  3. If there is low liquidity for saleToken, but a high liquidity for holdToken, you may end up with more holdToken after the swap, making cache.holdTokenBalance greater than cache.borrowedAmount.

Impact

Underflow leading to an UniV3 position unable to be borrowed

Code Snippet

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L116C5-L140C6
https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/abstract/LiquidityManager.sol#L150

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L869
https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L874

Tool used

Manual Review

Recommendation

Check which of the 2 variables is bigger than the other and adapt the calculation :

uint256 borrowingCollateral;
if (cache.borrowedAmount < cache.holdTokenBalance) {
    borrowingCollateral = /* some calculated or default value */;
    // OR for example uint256 borrowingCollateral = (cache.borrowedAmount * cache.holdTokenBalance) / totalHoldToken;
} else {
    borrowingCollateral = cache.borrowedAmount - cache.holdTokenBalance;
}

Duplicate of #86

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Oct 26, 2023
@sherlock-admin2 sherlock-admin2 changed the title Sticky Teal Sheep - Underflow in borrow() Function lil.eth - Underflow in borrow() Function Oct 30, 2023
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Oct 30, 2023
@Evert0x Evert0x added Medium A valid Medium severity issue and removed High A valid High severity issue labels Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants