wfCashERC4626 maxWithdraw, previewWithdraw, previewRedeem, convertToAssets, convertToShares doesn't conform to EIP4626 #167
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
disagree with severity
Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)
duplicate
This issue or pull request already exists
Lines of code
https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L85
https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L21
https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L23
https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L71
Vulnerability details
EIP4626 states that
maxWithdraw
,convertToAssets
,convertToShares
,previewRedeem
andpreviewWithdraw
must not revert (unless due to large input, or due to a reason that will makedeposit
/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
viatotalAssets
.getMaturedValue
will revert if the account has not been settled yet: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: