sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

0x52 - Protection buyer can game default protection by never redeeming principal from underlying LP token #245

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

0x52

high

Protection buyer can game default protection by never redeeming principal from underlying LP token

Summary

When locking tokens in the event of a default, each LP token is valued via an adapter. For the GoldFinch this valuation can be inflated by never redeeming any principal from the token. After the token has been valued the holder can double dip, by redeeming the principal from GoldFinch and then redeeming their insurance for the value of the token.

Vulnerability Detail

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/adapters/GoldfinchAdapter.sol#L170-L177

When calculating the remaining principal by subtracting the principal redeemed from the initial principal. As long as the token holder never redeems then their token will always be valued at the entire initial principal even if they have principal that can be claimed. This can be abused to double dip. When a default happens the insurance will entitle them to a payment for the initial payment. After their token has been valued, they can claim their redeemable interest as well as the insurance.

Impact

Protection buyer can double dip and take more insurance than expected

Code Snippet

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/adapters/GoldfinchAdapter.sol#L153-L179

Tool used

ChatGPT

Recommendation

Query the GoldFinch pool via redeemableInterestAndPrincipal to determine the amount of principal that's redeemable and remove that from the value of the token.

clems4ev3r commented 1 year ago

Looks like a duplicate of #8

vnadoda commented 1 year ago

@clems4ev3r This is not valid issue. Default payout is not implemented yet, but idea/plan is that when buyers claim payouts for defaults, they have to lock/transfer their LP tokens into Carapace vault. So they should not be able to claim twice.

Another point to keep in mind is that locked capital (which will be used for default payout) is calculated based on min(RemainingPrincipal, ProtectionAmount).

CC @taisukemino

clems4ev3r commented 1 year ago

@vnadoda, respectfully disagree, 0x52 states that remainingPrincipal is calculated wrongly and does not take in account amounts repaid by the borrower but not redeemed by the lender on Goldfinch. This inflates the amount the protection buyer can protect themselves against. Seems like a valid issue IMO, even without default payout implemented

vnadoda commented 1 year ago

@clems4ev3r 0x52's main concern is that there might be the redeemable principal and interest, which protection buyers will not redeem to game the carapace for higher coverage. But if buyers don't redeem and there is a default, that means they are entitled to remaining principal. Now on default, they can not redeem and claim payout at max because carapace will require LP token in lock/vault. So I don't see how buyers can double deep. 0x52 has raised interesting point and @taisukemino & I will discuss this further internally.

vnadoda commented 1 year ago

@clems4ev3r Can you review this issue based on my latest comment? I have discussed this internally with @taisukemino and the concern seems invalid.

clems4ev3r commented 1 year ago

@vnadoda, okay I understand your point now.

In this case a lot of the security concerns are solved by the default payout implementation which was not available during contest, so not sure how to classify here

vnadoda commented 1 year ago

@clems4ev3r this should be classified as informational. We will keep this in mind when we design/implement the default payout.

vnadoda commented 1 year ago

@hrishibhat can we close this?

hrishibhat commented 1 year ago

Closing based on the above comments