-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
xiaoming90 - Unexpected behavior when calling certain ERC4626 functions #43
Comments
Valid issue and I would consider increasing the severity to high. While this is not a risk for the leveraged vault framework (transactions revert inside the time window between fCash maturity and settlement rate initialization), this may cause severe issues for another protocol which relies on the I believe the proper solution here is to |
@jeffywu Since it is impossible to know how it will impact other protocols for wrong integrations, I will maintain is medium severity. |
#13) * fix: sherlock-audit/2023-12-notional-update-5-judging#43 post maturity valuation * fix: rounding issues
The protocol team fixed this issue in PR/commit notional-finance/wrapped-fcash#13. |
The Lead Senior Watson signed-off on the fix. |
xiaoming90
medium
Unexpected behavior when calling certain ERC4626 functions
Summary
Unexpected behavior could occur when certain ERC4626 functions are called during the time windows when the fCash has matured but is not yet settled.
Vulnerability Detail
When the fCash has matured, the global settlement does not automatically get executed. The global settlement will only be executed when the first account attempts to settle its own account. The code expects the
pr.supplyFactor
to return zero if the global settlement has not been executed yet after maturity.Per the comment at Line 215, the design of the
_getMaturedCashValue
function is that it expects that if fCash has matured AND the fCash has not yet been settled, thepr.supplyFactor
will be zero. In this case, the cash value will be zero.https://github.com/sherlock-audit/2023-12-notional-update-5/blob/main/wrapped-fcash/contracts/wfCashBase.sol#L215
During the time window where the fCash has matured, and none of the accounts triggered an account settlement, the
_getMaturedCashValue
function at Line 33 below will return zero, which will result in thetotalAssets()
function returning zero.https://github.com/sherlock-audit/2023-12-notional-update-5/blob/main/wrapped-fcash/contracts/wfCashERC4626.sol#L33
Impact
The
totalAssets()
function is utilized by key ERC4626 functions within the wrapper, such as the following functions. The side effects of this issue are documented below:convertToAssets
(Impact = returned value is always zero assets regardless of the inputs)convertToAssets
>previewRedeem
(Impact = returned value is always zero assets regardless of the inputs)convertToAssets
>previewRedeem
>maxWithdraw
(Impact = max withdrawal is always zero)convertToShares
(Impact = Division by zero error, Revert)convertToShares
>previewWithdraw
(Impact = Revert)In addition, any external protocol integrating with wfCash will be vulnerable within this time window as an invalid result (zero) is returned, or a revert might occur. For instance, any external protocol that relies on any of the above-affected functions for computing the withdrawal/minting amount or collateral value will be greatly impacted as the value before the maturity might be 10000, but it will temporarily reset to zero during this time window. Attackers could take advantage of this time window to perform malicious actions.
Code Snippet
https://github.com/sherlock-audit/2023-12-notional-update-5/blob/main/wrapped-fcash/contracts/wfCashBase.sol#L215
https://github.com/sherlock-audit/2023-12-notional-update-5/blob/main/wrapped-fcash/contracts/wfCashERC4626.sol#L33
Tool used
Manual Review
Recommendation
Document the unexpected behavior of the affected functions that could occur during the time windows when the fCash has matured but is not yet settled so that anyone who calls these functions is aware of them.
The text was updated successfully, but these errors were encountered: