code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Borrowing aToken rebase rate can compensate for the borrowing interest rate and even generate positive benefits #285

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L500-L576

Vulnerability details

before every operator of wise lending, it will call _updatePseudoTotalAmounts to update borrow interest amount to pool.If the pool token is aToken, it will only calculate the interest generated by borrowing amount, and don't take into the rebase increasing amount after borrowing and interest generated.In low borrow rate,increaing amount will exceed borrow interest,and borrower will get benefits from borrowing,it is unreasonable.The borrower can even use repeated deposits and borrows to add leverage and obtain more rebase increasing amounts.

Impact

When positive rebase token (aToken) is borrowed, the amount and interest of the rebase increase are not calculated.

Proof of Concept

before every operator of wise lending, it will call _cleanup function in _preparePool function to add rebase amount or falsely sent amount to pool,and _updatePseudoTotalAmounts to update borrow interest amount to pool.If the pool token is aToken, it will only calculate the interest generated by borrowing amount, and don't calculate the rebase increasing amount after borrowing and interest generated.In low borrow rate,increaing amount may exceed borrow interest,and borrower will get benefits from borrowing,it is unreasonable.

It use syncPool to update lending amount and borrow amount before pool position operation, and it call _syncPoolBeforeCodeExecution to _preparePool:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L103-L105

    modifier syncPool(
        address _poolToken
    ) {
        (
            uint256 lendSharePrice,
            uint256 borrowSharePrice
        ) = _syncPoolBeforeCodeExecution(
            _poolToken
        );

        _;

        _syncPoolAfterCodeExecution(
            _poolToken,
            lendSharePrice,
            borrowSharePrice
        );
    }

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L286-L288

    function _syncPoolBeforeCodeExecution(
        address _poolToken
    )
        private
        returns (
            uint256 lendSharePrice,
            uint256 borrowSharePrice
        )
    {
        _checkReentrancy();

        _preparePool(
            _poolToken
        );
    ......
    }

In _preparePool,it will call _cleanup to add rebase amount or falsely sent amount of pool token to gobal pool data and lending data:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L372-L384

    function _preparePool(
        address _poolToken
    )
        internal
    {
        _cleanUp(
            _poolToken
        );

        _updatePseudoTotalAmounts(
            _poolToken
        );
    }

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L298-L343

    function _cleanUp(
        address _poolToken
    )
        internal
    {
        _validateNonZero(
            lendingPoolData[_poolToken].totalDepositShares
        );

        uint256 amountContract = _getBalance(
            _poolToken
        );

        uint256 totalPool = globalPoolData[_poolToken].totalPool;
        uint256 bareToken = globalPoolData[_poolToken].totalBareToken;

        if (_checkCleanUp(amountContract, totalPool, bareToken)) {
            return;
        }

        unchecked {

            uint256 difference = amountContract - (
                totalPool + bareToken
            );

            uint256 allowedDifference = _getAllowedDifference(
                _poolToken
            );

            if (difference > allowedDifference) {

                _increaseTotalAndPseudoTotalPool(
                    _poolToken,
                    allowedDifference
                );

                return;
            }

            _increaseTotalAndPseudoTotalPool(
                _poolToken,
                difference
            );
        }
    }

and use _updatePseudoTotalAmounts to update borrow interest:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L500-L576

    function _updatePseudoTotalAmounts(
        address _poolToken
    )
        private
    {
        uint256 currentTime = block.timestamp;

        uint256 bareIncrease = borrowPoolData[_poolToken].borrowRate
            * (currentTime - timestampsPoolData[_poolToken].timeStamp)
            * borrowPoolData[_poolToken].pseudoTotalBorrowAmount
            + bufferIncrease[_poolToken];

        if (bareIncrease < PRECISION_FACTOR_YEAR) {
            bufferIncrease[_poolToken] = bareIncrease;

            _setTimeStamp(
                _poolToken,
                currentTime
            );

            return;
        }

        delete bufferIncrease[_poolToken];

        uint256 amountInterest = bareIncrease
            / PRECISION_FACTOR_YEAR;

        uint256 feeAmount = amountInterest
            * globalPoolData[_poolToken].poolFee
            / PRECISION_FACTOR_E18;

        _increasePseudoTotalBorrowAmount(
            _poolToken,
            amountInterest
        );

        _increasePseudoTotalPool(
            _poolToken,
            amountInterest
        );

        if (feeAmount == 0) {
            _setTimeStamp(
                _poolToken,
                currentTime
            );
            return;
        }

        uint256 feeShares = feeAmount
            * lendingPoolData[_poolToken].totalDepositShares
            / (lendingPoolData[_poolToken].pseudoTotalPool - feeAmount);

        if (feeShares == 0) {
            _setTimeStamp(
                _poolToken,
                currentTime
            );
            return;
        }

        _increasePositionLendingDeposit(
            FEE_MANAGER_NFT,
            _poolToken,
            feeShares
        );

        _increaseTotalDepositShares(
            _poolToken,
            feeShares
        );

        _setTimeStamp(
            _poolToken,
            currentTime
        );
    }

In _updatePseudoTotalAmounts,it calculates interest of aToken based on borrowing amount,and don't take into the rebase increasing amount and interest.

When borrow rate is lower than rebase rate of aToken,, borrower can borrowing to earn, I think it is unreasonable.And borrower can use deposit and borrow to add leverage,make this benefits higher if borrow rate is low.

Tool Used

foundary、vscode

Recommended Mitigation Steps

Taking into aToken rebase in borrowing interest and borrowing amount.

Assessed type

Other

GalloDaSballo commented 6 months ago

The idea of accounting for aToken rebase is worth reviewing, but the report is lacking in proof and POC

Foon256 commented 6 months ago

User are not borrowing aToken. The pool token are just parked into aave to earn the extra APY from Aave. When someone borrows from an aave pool, the aToken are withdrawn from Aave and send to the user. There are not aaveDebt token creating extra rebase. In other words user borrow just the underlying token which only has a borrow rate defined by the token borrow rate curve. This reduces the effective amount parked in Aave and has influence on the total extra APY from Aave which is a function of the pool utilization. This is also taken into account when computing the effecitve Aave pool APY and displayed on the UI correcetly, being a proof that this functionallity is intended.

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as primary issue

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-judge commented 6 months ago

trust1995 marked the issue as unsatisfactory: Invalid