sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

Kumpa - After the pool returns from the locked state, ```_getExchangeRate()``` may be broken and stop ```deposit()``` from functioning #256

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Kumpa

high

After the pool returns from the locked state, _getExchangeRate() may be broken and stop deposit() from functioning

Summary

When the pool is in lock state, totalSTokenUnderlying is set to zero but totalSupply() of SToken is not adjusted (or burn) to match the change in totalSTokenUnderlying. The result may cause _getExchangeRate() to always return zero, leading to sellers unable to deposit underlying token into the pool after the pool becomes active again after the locked state.

Vulnerability Detail

1.When the pool get lockedCapital(), if totalSTokenUnderlying is less than protection amount or remaining principal, then totalSTokenUnderlying is set to zero. https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L413-L422

2.totalSupply() of SToken, however, remains unchanged

3.If the pool becomes active again, no user will be able to deposit again because _getExchangeRate() will always return zero due to totalSTokenUnderlying equal to zero, causing convertToSToken, which relies on _getExchangeRate() to calculate the amount of SToken that user may receive, to revert.

The exchange rate will always be zero.

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L913-L930

Because totalSupply of SToken is not changed or burned to zero, convertToSToken() will then proceed to the calculation that relies on exchange rate which will always return zero. Since it is impossible to be divided by zero, convertToSToken will then revert.

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L589-L606

Impact

This vulnerability may not directly impact the withdrawal side of sellers because they can opt to withdraw their underlying token via claim function but it will break the deposit function of the contract which will affect the buyer side who face the risk of future default. If there is any default happens in the future, there will be no more SToken from sellers to payout to the buyer.

Code Snippet

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L413-L422 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L913-L930 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L589-L606

Tool used

Manual Review

Recommendation

totalSupply() of SToken should get updated whenever there is a change in totalSTokenUnderlying.

Duplicate of #117