search

sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

clems4ever - Protection buyer may buy multiple protections for same goldfinch NFT #112

Open github-actions[bot] opened 1 year ago

github-actions[bot] commented 1 year ago

clems4ever

high

Protection buyer may buy multiple protections for same goldfinch NFT

Summary

The Carapace protocol checks that a protection buyer does not buy a protection for an amount greater than the remainingPrincipal in the corresponding loan. However it possible for the buyer to buy multiple different protections for the same Goldfinch loan.

Vulnerability Detail

The check for the possibility for a user to buy a protection is done here in ReferenceLendingPools.canBuyProtection: https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ReferenceLendingPools.sol#L132-L168

It checks the protection about to be created does not cross remaining principal. But it still allows the user to create multiple protections for the same loan position.

Impact

The malicious user can overprotect their loan position on Goldfinch and thus claim a larger amount on loan default than what they lended. For now as the default claiming feature is not implemented, they can use this bug to DOS the protocol by using all funds deposited into the protocol reaching leverageRatioFloor and not allowing any new protections to be bought.

Code Snippet

Tool used

Manual Review

Recommendation

Keep track of the total protection subscribed for a given loan and limit total protection value to remaining capital

vnadoda commented 1 year ago

@clems4ev3r this is duplicate of #193 & #139

hrishibhat commented 1 year ago

Sponsor comment from #193:

Double buying of protections for the same NFT is a known issue and we were planning to tackle it in an upcoming version because even after buying multiple protections buyers won't be able to claim for the same position as default payout will require NFT lock/transfer in the carapace vault.

This double counting of locked capital issue seems a legit concern. Now we are considering fixing this with other audit issues.

vnadoda commented 1 year ago

@clems4ev3r PR for this fix: https://github.com/carapace-finance/credit-default-swaps-contracts/pull/59

clems4ev3r commented 1 year ago

Fix looks good