sherlock-audit / 2024-04-teller-finance-judging

6 stars 6 forks source link

pkqs90 - `STANDARD_EXPANSION_FACTOR` is not needed - required collateral amount is 1e18 times smaller than it should be. #261

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

pkqs90

high

STANDARD_EXPANSION_FACTOR is not needed - required collateral amount is 1e18 times smaller than it should be.

Summary

When calculating the requiredCollateral for a loan, it is multipled by STANDARD_EXPANSION_FACTOR (1e18). This is not required, and the current implementation incorrectly allows 1e18 times smaller collateral amount to create a loan.

Vulnerability Detail

In acceptFundsForAcceptBid, the _collateralAmount is expanded by STANDARD_EXPANSION_FACTOR (1e18) when comparing with requiredCollateral. This is incorrect.

By reading the code comments, it says that _getPriceFromSqrtX96() function returns a price ratio expanded by 1e18, which is incorrect. What this function does is calculating the _sqrtPriceX96 * _sqrtPriceX96 / 2**96, which is the actualPrice * 2**96. The 2**96 already serves as the precision factor, and the 1e18 is simply incorrect.

    function acceptFundsForAcceptBid(
        address _borrower,
        uint256 _bidId,
        uint256 _principalAmount,
        uint256 _collateralAmount,
        address _collateralTokenAddress,
        uint256 _collateralTokenId, 
        uint32 _loanDuration,
        uint16 _interestRate
    ) external onlySmartCommitmentForwarder whenNotPaused {

        require(
            _collateralTokenAddress == address(collateralToken),
            "Mismatching collateral token"
        );
        //the interest rate must be at least as high has the commitment demands. The borrower can use a higher interest rate although that would not be beneficial to the borrower.
        require(_interestRate >= getMinInterestRate(), "Invalid interest rate");
        //the loan duration must be less than the commitment max loan duration. The lender who made the commitment expects the money to be returned before this window.
        require(_loanDuration <= maxLoanDuration, "Invalid loan max duration");

        require(
            getPrincipalAmountAvailableToBorrow() >= _principalAmount,
            "Invalid loan max principal"
        );

        //this is expanded by 10**18
>       uint256 requiredCollateral = getCollateralRequiredForPrincipalAmount(
>           _principalAmount
>       );

        require(
            (_collateralAmount * STANDARD_EXPANSION_FACTOR) >=
                requiredCollateral,
            "Insufficient Borrower Collateral"
        );

        ...
    }
    //this result is expanded by UNISWAP_EXPANSION_FACTOR
    function _getUniswapV3TokenPairPrice(uint32 _twapInterval)
        internal
        view
        returns (uint256)
    {
        // represents the square root of the price of token1 in terms of token0

        uint160 sqrtPriceX96 = getSqrtTwapX96(_twapInterval);

        //this output is the price ratio expanded by 1e18
>       return _getPriceFromSqrtX96(sqrtPriceX96);
    }

    //this result is expanded by UNISWAP_EXPANSION_FACTOR
    function _getPriceFromSqrtX96(uint160 _sqrtPriceX96)
        internal
        pure
        returns (uint256 price_)
    {

        uint256 priceX96 = (uint256(_sqrtPriceX96) * uint256(_sqrtPriceX96)) /
            (2**96);

        // sqrtPrice is in X96 format so we scale it down to get the price
        // Also note that this price is a relative price between the two tokens in the pool
        // It's not a USD price
        price_ = priceX96;
    }

    // ---- TWAP

    function getSqrtTwapX96(uint32 twapInterval)
        public
        view
        returns (uint160 sqrtPriceX96)
    {
        if (twapInterval == 0) {
            // return the current price if twapInterval == 0
            (sqrtPriceX96, , , , , , ) = IUniswapV3Pool(UNISWAP_V3_POOL)
                .slot0();
        } else {
            uint32[] memory secondsAgos = new uint32[](2);
            secondsAgos[0] = twapInterval; // from (before)
            secondsAgos[1] = 0; // to (now)

            (int56[] memory tickCumulatives, ) = IUniswapV3Pool(UNISWAP_V3_POOL)
                .observe(secondsAgos);

            // tick(imprecise as it's an integer) to price
            sqrtPriceX96 = TickMath.getSqrtRatioAtTick(
                int24(
                    (tickCumulatives[1] - tickCumulatives[0]) /
                        int32(twapInterval)
                )
            );
        }
    }

Impact

Allows borrowers to borrow a loan with 1e18 times smaller of required collateral amount.

Code Snippet

Tool used

Manual review

Recommendation

Remove STANDARD_EXPANSION_FACTOR.

Duplicate of #58