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

1 stars 1 forks source link

M-14 Unmitigated #42

Closed c4-bot-4 closed 4 months ago

c4-bot-4 commented 4 months ago

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Vault.sol#L345

Vulnerability details

C4 issue

M-14: V3Vault is not ERC-4626 compliant

Comment

The original contract V3Vault is not ERC4626 compliant because functions maxRedeem, maxMint, maxWithdraw, maxDeposit don't return the correct value.

Proof of concept

The fix for this issue is PR #15 The mitigation updated the above 4 functions to reflect the correct values; however, there is 1 mistake at function maxWithdraw:

function maxWithdraw(address owner) external view override returns (uint256) {
        (uint256 debtExchangeRateX96, uint256 lendExchangeRateX96) = _calculateGlobalInterest();

        uint256 ownerShareBalance = balanceOf(owner);
        uint256 ownerAssetBalance = _convertToAssets(ownerShareBalance, lendExchangeRateX96, Math.Rounding.Down);

        (uint256 balance,) = _getBalanceAndReserves(debtExchangeRateX96, lendExchangeRateX96);
        if (balance > ownerAssetBalance) {
            return ownerAssetBalance;
        } else {
            return _convertToAssets(balance, lendExchangeRateX96, Math.Rounding.Down);
        }
    }

In case balance <= ownerAssetBalance, while the functions should return balance, the balance get converted to assets and returned (the balance is of type asset itself, not share).

Tool used

Manual review

Recommended Mitigation Steps

Return balance instead.

Assessed type

Math

c4-judge commented 4 months ago

jhsagd76 marked the issue as new finding

c4-judge commented 4 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 4 months ago

jhsagd76 marked the issue as duplicate of #63