withdrawlRequests and totalSTokenRequested are not updated when sTokens are transferred
Summary
When users transfer sTokens and new deposits are made withdrawlRequests and totalSTokenRequested are not updated to reflect the fact that those requested are not viable. This leads to many issues. Protection buyers can not trust the total withdrawlRequested amount since it could be inflated.
This also makes it possible for users to withdraw at the end of each cycle even if they deposit in the same cycle.
Vulnerability Detail
If a seller requests a withdraw with the requestWithdrawal() function and then sells the sTokens the
withdrawlRequests and totaSTokenRequested are not changed. Another user could buy the tokens
and also request a withdraw, this would inflate the totalSTokenRequested to an amount that is above actual amount that can be withdrawn.
Impact
The totalSTokenRequested is key component for protection buyers when they are assessing if a protection premium they are willing to pay for and the insolvency risk. From the documentation we can read the following:
Let’s use an example where the cycle duration is 90 days. Imagine that Buyer A buys protection at day 100 (cycle 2) for 170 days. She can see how much capital will stay during her protection duration (the remaining days in cycle 2 and all the days in cycle 3) from two pieces of information. First, all the capital deposited in cycle 2 will stay in cycle 3. Second, the amount of withdrawal requests made before cycle 1 (only in cycle 1 in this case)."
Since totalSTokenRequested can be inflated protection buys will not be able to correctly analyze the ProtectionPool. They will base their decisions on false information. With a large enough inflation buyers will likely not buy any protection.
There is also another consequence of this vulnerability. Since sTokens can be traded freely and withdrawal requested can be made on multiple accounts user could craft a strategy where they will be able to deposit at any point during a cycle and exit at the end.
I will outline how this can be done:
A user either buys or deposit to acquire sTokens in cycle n
They request a withdrawal for the end of cycle n+2
They transfer those sTokens to another account and do the same there.
Repeat step 2-3 for multiple accounts.
In cycle n+1 they do the same thing again. They continue doing this in every cycle.
For every cycle >= n+2 the user can deposit to any of his accounts at any time during the cycle and exit at the end.
This completely breaks the assumption that capital deposited in a cycle will stay in the next cycle as outlined in the documentation. The capital that should exist to pay for default can be withdrawn when it should not be. The result is that insolvency can happen when it should not since large amount of capital can leave the protectionpool when it should have been withdrawable.
monrel
high
withdrawlRequests and totalSTokenRequested are not updated when sTokens are transferred
Summary
When users transfer sTokens and new deposits are made
withdrawlRequestsandtotalSTokenRequestedare not updated to reflect the fact that those requested are not viable. This leads to many issues. Protection buyers can not trust the total withdrawlRequested amount since it could be inflated.This also makes it possible for users to withdraw at the end of each cycle even if they deposit in the same cycle.
Vulnerability Detail
If a seller requests a withdraw with the
requestWithdrawal()function and then sells the sTokens thewithdrawlRequestsandtotaSTokenRequestedare not changed. Another user could buy the tokens and also request a withdraw, this would inflate thetotalSTokenRequestedto an amount that is above actual amount that can be withdrawn.Impact
The
totalSTokenRequestedis key component for protection buyers when they are assessing if a protection premium they are willing to pay for and the insolvency risk. From the documentation we can read the following:Let’s use an example where the cycle duration is 90 days. Imagine that Buyer A buys protection at day 100 (cycle 2) for 170 days. She can see how much capital will stay during her protection duration (the remaining days in cycle 2 and all the days in cycle 3) from two pieces of information. First, all the capital deposited in cycle 2 will stay in cycle 3. Second, the amount of withdrawal requests made before cycle 1 (only in cycle 1 in this case)."
Since
totalSTokenRequestedcan be inflated protection buys will not be able to correctly analyze the ProtectionPool. They will base their decisions on false information. With a large enough inflation buyers will likely not buy any protection.There is also another consequence of this vulnerability. Since sTokens can be traded freely and withdrawal requested can be made on multiple accounts user could craft a strategy where they will be able to deposit at any point during a cycle and exit at the end.
I will outline how this can be done:
This completely breaks the assumption that capital deposited in a cycle will stay in the next cycle as outlined in the documentation. The capital that should exist to pay for default can be withdrawn when it should not be. The result is that insolvency can happen when it should not since large amount of capital can leave the protectionpool when it should have been withdrawable.
Code Snippet
Each request for withdrawal will inflate the requested amount without every decreasing it https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L1061-L1097
Tool used
Manual Review, vscode, foundry
Recommendation
Transfers of sTokens should remove the appropriate amount of
withdrawlRequestsandtotaSTokenRequested.Duplicate of #292