code-423n4 / 2024-04-revert-mitigation-findings

1 stars 1 forks source link

M-08 MitigationConfirmed #13

Open c4-bot-8 opened 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

Vulnerability details

C4 issue

M-08: DailyLendIncreaseLimitLeft and dailyDebtIncreaseLimitLeft are not adjusted accurately

Comment

In the original implementation, sometimes when dailyLendIncreaseLimitLeft/dailyDebtIncreaseLimitLeft changes value, they are not proceeded by a call to _resetDailyLendIncreaseLimit/_resetDailyDebtIncreaseLimit:

function _resetDailyDebtIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily debt limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyDebtIncreaseLimitLastReset) {
            uint256 debtIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
                * (Q32 + MAX_DAILY_DEBT_INCREASE_X32) / Q32;
            dailyDebtIncreaseLimitLeft =
                dailyDebtIncreaseLimitMin > debtIncreaseLimit ? dailyDebtIncreaseLimitMin : debtIncreaseLimit;
            dailyDebtIncreaseLimitLastReset = time;
        }
    }

Consequently, these changes to dailyLendIncreaseLimitLeft/dailyDebtIncreaseLimitLeft will not be reflected in a new day, because in new day _resetDailyLendIncreaseLimit/_resetDailyDebtIncreaseLimit will just discard these changes.

Mitigation

PR #11 The mitigation code will calls _resetDailyLendIncreaseLimit/_resetDailyDebtIncreaseLimit every time dailyLendIncreaseLimitLeft/dailyDebtIncreaseLimitLeft is updated. For example in _withdraw function:

function _withdraw(address receiver, address owner, uint256 amount, bool isShare)
        internal
        returns (uint256 assets, uint256 shares)
    {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();
        _resetDailyLendIncreaseLimit(newLendExchangeRateX96, false);

        if (isShare) {
            shares = amount;
            assets = _convertToAssets(amount, newLendExchangeRateX96, Math.Rounding.Down);
        } else {
            assets = amount;
            shares = _convertToShares(amount, newLendExchangeRateX96, Math.Rounding.Up);
        }

        uint256 ownerBalance = balanceOf(owner);
        if (shares > ownerBalance) {
            shares = ownerBalance;
            assets = _convertToAssets(amount, newLendExchangeRateX96, Math.Rounding.Down);
        }

        // if caller has allowance for owners shares - may call withdraw
        if (msg.sender != owner) {
            _spendAllowance(owner, msg.sender, shares);
        }

        (uint256 balance,) = _getBalanceAndReserves(newDebtExchangeRateX96, newLendExchangeRateX96);
        if (balance < assets) {
            revert InsufficientLiquidity();
        }

        // fails if not enough shares
        _burn(owner, shares);
        SafeERC20.safeTransfer(IERC20(asset), receiver, assets);

        // when amounts are withdrawn - they may be deposited again
        dailyLendIncreaseLimitLeft = dailyLendIncreaseLimitLeft + assets;

        emit Withdraw(msg.sender, receiver, owner, assets, shares);
    }

The mitigation solved the original issue.

Conclusion

LGTM

c4-judge commented 4 months ago

jhsagd76 marked the issue as satisfactory