sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

Kumpa - Malicious users doubly earn their underlying token when the pool returns from locked state #255

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Kumpa

high

Malicious users doubly earn their underlying token when the pool returns from locked state

Summary

claimUnlockedCapital(), which allows seller to claim his underlying token after the pool _moveFromLockedToActiveState(), does not remove sToken from user balance so user may be able to requestWithdrawal() his underlying token again even though he already claims unlocked underlying token.

Vulnerability Detail

1.When the pool _moveFromLockedToActiveState(), the seller is able to claimUnlockedCapital() to get his underlying token back https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L427-L445

2.claimableAmount is based on balanceOf sToken at the snapshot time. However, in claiming function, there is no function that neither removes nor burns sToken in seller’s balance even after the underlying token is claimed. https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L158-L201 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L453-L521

3.In requestWithdrawal(), the function checks balanceOf(sellers) to determine if seller is able to withdraw the underlying token and the amount that he is able to get. Since sToken is not removed from user’s balance when claiming, seller is able to regain the underlying token again. https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L1061-L1065

Impact

Underlying token will be drained by malicious sellers and if token is fully drained, some seller may not be able to claim or withdraw his token. This may also affect payout mechanism.

Code Snippet

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L427-L445 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L158-L201 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L453-L521 https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L1061-L1065

Tool used

Manual Review

Recommendation

The team should consider adding burn function to remove seller’s sToken during claiming process.

Duplicate of #210