sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

__141345__ - Buyers could buy multiple times #285

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

141345

medium

Buyers could buy multiple times

Summary

There is no mechanism to prevent buyers to submit the same buy order multiple times. With the same LP token, buyers could duplicate buying protections and lose premium.

Vulnerability Detail

In case of network congestion, or user mistakes, the buy protection transaction could be sent more than once, however, currently the system is not handling the duplicate buy orders. As a result, buyers could pay twice the premium and lose the fund of extra payment.

In function _verifyAndCreateProtection(), the first verification is to check whether the buying order is legit.

File: contracts/core/pool/ProtectionPool.sol
795:   function _verifyAndCreateProtection(

802:     ProtectionPoolHelper.verifyProtection(
803:       poolCycleManager,
804:       defaultStateManager,
805:       address(this),
806:       poolInfo,
807:       _protectionStartTimestamp,
808:       _protectionPurchaseParams,
809:       _isRenewal
810:     );

The check for the buyer side is through calling function verifyProtection():

File: contracts/libraries/ProtectionPoolHelper.sol
36:   function verifyProtection(

69:     if (
70:       !poolInfo.referenceLendingPools.canBuyProtection(
71:         msg.sender,
72:         _protectionPurchaseParams,
73:         _isRenewal
74:       )
75:     ) {
76:       revert IProtectionPool.ProtectionPurchaseNotAllowed(
77:         _protectionPurchaseParams
78:       );
79:     }
80:   }

But in the verification function canBuyProtection(), it only verifies that the LP token principal amount and protection amount.

File: contracts/core/pool/ReferenceLendingPools.sol
132:   function canBuyProtection(
133:     address _buyer,
134:     ProtectionPurchaseParams calldata _purchaseParams,
135:     bool _isRenewal
136:   )
137:     external
138:     view
139:     override
140:     whenLendingPoolSupported(_purchaseParams.lendingPoolAddress)
141:     returns (bool)
142:   {

159:     /// Verify that protection amount is less than or equal to the remaining principal
160:     /// that buyer has lent to the underlying lending pool
161:     return
162:       _purchaseParams.protectionAmount <=
163:       calculateRemainingPrincipal(
164:         _purchaseParams.lendingPoolAddress,
165:         _buyer,
166:         _purchaseParams.nftLpTokenId
167:       );
168:   }

Imagine, Alice has LP token with principal of $2000 USDC, and buying the protection for $2000, the above verification can go through multiple times. If Alice send the buy order more than once, whether due to network issue or manual mistakes, she would be charged twice the premium, the extra premium will be lost.

Impact

Buyers could lose fund unexpectedly in case of network problem or user mistakes.

Code Snippet

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

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/libraries/ProtectionPoolHelper.sol#L69-L80

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ReferenceLendingPools.sol#L159-L168

Tool used

Manual Review

Recommendation

In the verification functions, add some record of the LP token under protection. If some specific LP token is already under protection, just revert to prevent duplicate buying.

Or transfer the LP token at the time of buying, preventing the LP token to be used again.

Duplicate of #112