hats-finance / Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd

Other
0 stars 0 forks source link

The `buyCollateral` and `sellCollateral` Does not using latest borrow data #16

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @amankakar Twitter username: -- Submission hash (on-chain): 0x6c0bbd63a67344aeeb735e5a6f1a54d2843d44131fc716fd3f898c43f803d9e3 Severity: high

Description: Description\ The sellCollateral and buyCollateral function allows the user of Tapioca Dao to sell collateral to repay the debt or buy collateral to borrow more. However when user calls this function the user will pay debt with debt rate and interest . How ever it calls penrose.reAccrueBigBangMarkets() to update the totalBorrow with latest debt rate but the totalBorrow will not be update if market is bigBangEthMarket for more detail view lets break it down in step: The Sell Collateral function :

/home/aman/Desktop/audits/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/contracts/markets/bigBang/BBLeverage.sol:133
133:     function sellCollateral(address from, uint256 share, bytes calldata data)
134:         external
135:         optionNotPaused(PauseType.LeverageSell)
136:         solvent(from)
137:         notSelf(from)
138:         returns (uint256 amountOut)
139:     {
140:         if (address(leverageExecutor) == address(0)) {
141:             revert LeverageExecutorNotValid();
142:         }
143:         penrose.reAccrueBigBangMarkets(); <---- 
144: 

The Buy Collateral function :

 function buyCollateral(address from, uint256 borrowAmount, uint256 supplyAmount, bytes calldata data)
        external
        optionNotPaused(PauseType.LeverageBuy)
        solvent(from)
        notSelf(from)
        returns (uint256 amountOut)
    {
        if (address(leverageExecutor) == address(0)) {
            revert LeverageExecutorNotValid();
        }
        penrose.reAccrueBigBangMarkets();

only bigBangEthMarket can call this it means from where we are calling this is bigBangEthMarket market.

function reAccrueBigBangMarkets() external notPaused {
        if (msg.sender == bigBangEthMarket) { // Only bigbang market can call this 
            _reAccrueMarkets(false);
        }
    }

From following code it can be seen that the accrue will only be called when market!=bigBangEthMarket and includeMainMarket is false but in our case it is Because if it is not we were not be reach this LOC.

function _reAccrueMarkets(bool includeMainMarket) private {
        uint256 len = allBigBangMarkets.length;
        address[] memory markets = allBigBangMarkets;
        for (uint256 i; i < len; i++) {
            address market = markets[i];
            if (isMarketRegistered[market]) {
                if (includeMainMarket || market != bigBangEthMarket) {
                    IBigBang(market).accrue();
                }
            }
        }

        emit ReaccruedMarkets(includeMainMarket);
    }

Now let have a look at _accure() function:

function _accrue() internal override {
        // accrue ETH market first
        {
            address ethMarket = penrose.bigBangEthMarket();
            if (ethMarket != address(this) && ethMarket != address(0)) {
                IBigBang(ethMarket).accrue();
            }
        }

        IBigBang.AccrueInfo memory _accrueInfo = accrueInfo;
        // Number of seconds since accrue was called
        uint256 elapsedTime = block.timestamp - _accrueInfo.lastAccrued;
        if (elapsedTime == 0) {
            return;
        }

        //update debt rate
        uint256 annumDebtRate = getDebtRate();
        _accrueInfo.debtRate = (annumDebtRate / 31557600).toUint64(); //per second; account for leap years
        _accrueInfo.lastAccrued = block.timestamp.toUint64();

        Rebase memory _totalBorrow = totalBorrow;

        // Calculate fees
        uint256 extraAmount = 0;
        extraAmount = (uint256(_totalBorrow.elastic) * _accrueInfo.debtRate * elapsedTime) / 1e18;

        // cap `extraAmount` to avoid overflow risk when converting it from uint256 to uint128
        uint256 max = type(uint128).max - totalBorrow.elastic;

        if (extraAmount > max) {
            extraAmount = max;
        }
        _totalBorrow.elastic += extraAmount.toUint128();
        openInterestDebt += extraAmount;

        totalBorrow = _totalBorrow;
        accrueInfo = _accrueInfo;

        emit LogAccrue(extraAmount, _accrueInfo.debtRate);
    }

This function is responsiable to update the debt rate , openInterestDebt ,and totalBorrow elastic values and these values are using by sellCollateral and buyCollateral function as we will deducted the borrow amount according to new debt rate.

Attack Scenario\ The User will be able to sell and buy collateral on old debt rate.

Matigation\ Also add calls to _accure() function.

cryptotechmaker commented 4 months ago

Invalid. Already called in the solvent modifier