sherlock-audit / 2024-02-tapioca-judging

3 stars 2 forks source link

hyh - Leverage operations of ETH market change debt, but do not accrue linked BB markets, corrupting their interest rate accrual logic #145

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

hyh

high

Leverage operations of ETH market change debt, but do not accrue linked BB markets, corrupting their interest rate accrual logic

Summary

BBLeverage of ETH market calls accrue before changing the debt with buyCollateral() or sellCollateral(), but doesn't invoke penrose.reAccrueBigBangMarkets(). It will corrupt the interest rate logic of the linked BB markets.

Vulnerability Detail

BB markets have interest rate logic linked to the utilization of the main ETH market. That's why the accrual needs to be performed for them each time ETH market debt changes (they might have the rate changed due to that and the accrual with old rate needs to be reflected in their accounting). It's not done in ETH market BBLeverage buyCollateral() and sellCollateral().

Impact

Linked markets will have the interest rate accrued incorrectly, namely old rate will be used for a longer period that it has to be according to the system logic. The period between ETH market debt change to the next accrual operation will have old rate in the linked BB markets, while it has to be new rate already. It will be a loss for lenders or borrowers depending on the direction of the change.

Likelihood: Medium + Impact: High = Severity: High.

Code Snippet

solvent modifier calls accrue operation only for the current market:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/Market.sol#L162-L170

    /// @dev Checks if the user is solvent in the closed liquidation case at the end of the function body.
    modifier solvent(address from, bool liquidation) {
        updateExchangeRate();
>>      _accrue();

        _;

        require(_isSolvent(from, exchangeRate, liquidation), "Market: insolvent");
    }

This way the issue of not updating the linked markets on accrual, while being fixed for borrow/repay and liquidations, is still in place for BBLeverage:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L53-L91

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

        ...

        {
>>          (, uint256 borrowShare) = _borrow(
                calldata_.from,
                address(this),
                calldata_.borrowAmount,
                _computeVariableOpeningFee(calldata_.borrowAmount)
            );
            (memoryData.borrowShareToAmount,) =
                yieldBox.withdraw(assetId, address(this), address(leverageExecutor), 0, borrowShare);
        }

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L126-L161

    function sellCollateral(address from, uint256 share, bytes calldata data)
        external
        optionNotPaused(PauseType.LeverageSell)
>>      solvent(from, false)
        notSelf(from)
        returns (uint256 amountOut)
    {
        if (address(leverageExecutor) == address(0)) {
            revert LeverageExecutorNotValid();
        }
        _allowedBorrow(from, share);
        _removeCollateral(from, address(this), share);

        ...

        memoryData.partOwed = userBorrowPart[from];
        memoryData.amountOwed = totalBorrow.toElastic(memoryData.partOwed, true);
        memoryData.shareOwed = yieldBox.toShare(assetId, memoryData.amountOwed, true);
        if (memoryData.shareOwed <= memoryData.shareOut) {
>>          _repay(from, from, memoryData.partOwed);
        } else {
            //repay as much as we can
            uint256 partOut = totalBorrow.toBase(amountOut, false);
>>          _repay(from, from, partOut);
        }

Tool used

Manual Review

Recommendation

Consider calling the linked markets update, e.g.:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L53-L62

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

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L126-L137

    function sellCollateral(address from, uint256 share, bytes calldata data)
        external
        optionNotPaused(PauseType.LeverageSell)
        solvent(from, false)
        notSelf(from)
        returns (uint256 amountOut)
    {
        if (address(leverageExecutor) == address(0)) {
            revert LeverageExecutorNotValid();
        }
+       penrose.reAccrueBigBangMarkets();
        _allowedBorrow(from, share);
        _removeCollateral(from, address(this), share);

Duplicate of #128

cryptotechmaker commented 4 months ago

Duplicate of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/128