sherlock-audit / 2024-02-leverage-contracts-judging

1 stars 0 forks source link

FastTiger - There is no function to refund the borrower for the remaining position holding cost in the repay function. #22

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

FastTiger

high

There is no function to refund the borrower for the remaining position holding cost in the repay function.

Summary

Since repay function does not have a function to refund the borrower for the remaining position holding cost, the borrower loses.

Vulnerability Detail

LiquidityBorrowingManager.sol#repay used for repaying loans, optionally with liquidation or emergency liquidity withdrawal. However repay function does not have a function to refund the borrower for the remaining position holding cost when the moment of liquidation has not arrived.

    function repay(
        RepayParams calldata params,
        uint256 deadline
    )
        external
        nonReentrant
        checkDeadline(deadline)
        returns (uint256 saleTokenOut, uint256 holdTokenOut)
    {

        ...

        // Check if it's an emergency repayment
        if (params.isEmergency) {

            ...

        } else {
            // Deduct borrowedAmount from totalBorrowed
            holdTokenRateInfo.totalBorrowed -= borrowing.borrowedAmount;

            // Transfer the borrowed amount and liquidation bonus from the VAULT to this contract
676         Vault(VAULT_ADDRESS).transferToken(
677             borrowing.holdToken,
678             address(this),
679             borrowing.borrowedAmount + liquidationBonus
680         );

            if (params.externalSwap.length != 0) {
                _callExternalSwap(borrowing.holdToken, params.externalSwap);
            }

            // Restore liquidity using the borrowed amount and pay a daily rate fee
            LoanInfo[] memory loans = loansInfo[params.borrowingKey];
            _maxApproveIfNecessary(
                borrowing.holdToken,
                address(underlyingPositionManager),
                type(uint128).max
            );
            _maxApproveIfNecessary(
                borrowing.saleToken,
                address(underlyingPositionManager),
                type(uint128).max
            );

            _restoreLiquidity(
                RestoreLiquidityParams({
                    zeroForSaleToken: zeroForSaleToken,
                    swapPoolfeeTier: params.internalSwapPoolfee,
                    totalfeesOwed: borrowing.feesOwed,
                    totalBorrowedAmount: borrowing.borrowedAmount
                }),
                loans
            );

            // Remove borrowing key from related data structures
            _removeKeysAndClearStorage(borrowing.borrower, params.borrowingKey, loans);

            // Get the remaining balance of saleToken and holdToken
            (saleTokenOut, holdTokenOut) = _getPairBalance(
                borrowing.saleToken,
                borrowing.holdToken
            );

            if (saleTokenOut > 0 && params.returnOnlyHoldToken) {
                (, uint256 holdTokenAmountOut) = _simulateSwap(
                    zeroForSaleToken,
                    params.internalSwapPoolfee,
                    borrowing.saleToken, // saleToken is tokenIn
                    borrowing.holdToken,
                    saleTokenOut
                );
                if (holdTokenAmountOut > 0) {
                    // Call the internal v3SwapExactInput function
                    holdTokenOut += _v3SwapExactInput(
                        v3SwapExactInputParams({
                            fee: params.internalSwapPoolfee,
                            tokenIn: borrowing.saleToken,
                            tokenOut: borrowing.holdToken,
                            amountIn: saleTokenOut
                        })
                    );
                    saleTokenOut = 0;
                }
            }

            (holdTokenOut < params.minHoldTokenOut || saleTokenOut < params.minSaleTokenOut)
                .revertError(ErrLib.ErrorCode.PRICE_SLIPPAGE_CHECK);

            // Pay a profit to a msg.sender
744         _pay(borrowing.holdToken, address(this), msg.sender, holdTokenOut);
745         _pay(borrowing.saleToken, address(this), msg.sender, saleTokenOut);

            emit Repay(borrowing.borrower, msg.sender, params.borrowingKey);
        }
    }

Therefore the borrower loses the remaining position holding cost.

Impact

The borrower loses the remaining position holding cost.

Code Snippet

https://github.com/sherlock-audit/2024-02-leverage-contracts/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L671C9-L748C10

Tool used

Manual Review

Recommendation

Add to follow lines in LiquidityBorrowingManager.sol#repay function.

    function repay(
        RepayParams calldata params,
        uint256 deadline
    )
        external
        nonReentrant
        checkDeadline(deadline)
        returns (uint256 saleTokenOut, uint256 holdTokenOut)
    {

        ...

        // Check if it's an emergency repayment
        if (params.isEmergency) {

            ...

        } else {
            // Deduct borrowedAmount from totalBorrowed
            holdTokenRateInfo.totalBorrowed -= borrowing.borrowedAmount;

            // Transfer the borrowed amount and liquidation bonus from the VAULT to this contract
676         Vault(VAULT_ADDRESS).transferToken(
677             borrowing.holdToken,
678             address(this),
679             borrowing.borrowedAmount + liquidationBonus
680         );

            ...
            // Pay a profit to a msg.sender
744         _pay(borrowing.holdToken, address(this), msg.sender, holdTokenOut);
745         _pay(borrowing.saleToken, address(this), msg.sender, saleTokenOut);

++          if(collateralBalance > 0)
++              _pay(borrowing.holdToken, address(Vault), msg.sender, (uint256)collateralBalance);

            emit Repay(borrowing.borrower, msg.sender, params.borrowingKey);
        }
    }
nevillehuang commented 8 months ago

Invalid, borrower should repay whatever he currently owes, so this would constitue user input error not accepted based on sherlock rules