Lines of code
<https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L21&gt;
<https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L23&gt;
<https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L71&gt;

Vulnerability details

EIP4626 states that maxWithdraw, convertToAssets, convertToShares, previewRedeem and previewWithdraw must not revert (unless due to large input, or due to a reason that will make deposit/redeem revert).
However, wfCash4626’s implementation of those ends up calling _getMaturedValue if the asset has matured.
This call will revert if the account has not been settled yet.

Impact

Incorrect implementation of EIP4626.
Protocols that will build upon wfCash4626 may revert unexpectedly.

Proof of Concept

All these functions end up calling _getMaturedValue via totalAssets.
getMaturedValue will revert if the account has not been settled yet:

        // We cannot settle an account in a view method, so this may fail if the account has not been settled
        // after maturity. This can be done by anyone so it should not be an issue
        (int256 cashBalance, /* */, /* */) = NotionalV2.getAccountBalance(currencyId, address(this));
        int256 underlyingExternal = NotionalV2.convertCashBalanceToExternal(currencyId, cashBalance, true);
        require(underlyingExternal > 0, "Must Settle");

Therefore, the functions don’t implement EIP4626 correctly, and integration with wfCash might fail.

Recommended Mitigation Steps

Create a view method that will simulate the settlement of an account and get the cash balance of the account’s currencyId. Then use that cashBalance to calculate the underlyingExternal.

The text was updated successfully, but these errors were encountered:

👀 1 berndartmueller reacted with eyes emoji

All reactions

• 👀 1 reaction