0x52 - The renewal grace period gives users insurance for no premium

[github-actions[bot]]

0x52

medium

The renewal grace period gives users insurance for no premium

Summary

When a protection position is renewed, the contract checks that the expired timestamp is within the grace period of the current timestamp. The issue is that when it is renewed, it starts insurance at block.timestamp rather than the expiration of the previous protection. The result is that the grace period is effectively free insurance for the user.

Vulnerability Detail

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

When checking if a position can be renewed it checks the expiration of the previous protection to confirm that it is being renewed within the grace period.

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

After checking if the protection can be removed it starts the insurance at block.timestamp. The result is that the grace period doesn't collect any premium for it's duration. To abuse this the user would keep renewing at the end of the grace period for the shortest amount of time so that they would get the most amount of insurance for free.

One might argue that the buyer didn't have insurance during this time but protection can be renewed at any time during the grace period and late payments are very easy to see coming (i.e. if the payment is due in 30 days and it's currently day 29). The result is that even though technically there isn't insurance the user is still basically insured because they would always be able to renew before a default.

Impact

Renewal grace period can be abused to get free insurance

Code Snippet

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

Tool used

ChatGPT

Recommendation

When renewing protection, the protection should renew from the end of the expired protection not block.timestamp.

[clems4ev3r]

looks like a duplicate of #179

[vnadoda]

@clems4ev3r actually this is a duplicate of #190

[clems4ev3r]

@vnadoda agreed, as per my comment on #190:

190 #308 and #179 are duplicates

[IAm0x52]

Escalate for 50 USDC

Given the yield of the sellers is directly harmed by this and there are no prerequisites for this to happen, I believe that this should be high risk. I would like to point out that #179 and #190 already categorize this as high.

[sherlock-admin]

Escalate for 50 USDC

Given the yield of the sellers is directly harmed by this and there are no prerequisites for this to happen, I believe that this should be high risk. I would like to point out that #179 and #190 already categorize this as high.

You've created a valid escalation for 50 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

Considering this issue as high

[sherlock-admin]

Escalation accepted

Considering this issue as high

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

[vnadoda]

@clems4ev3r @hrishibhat PR for this issue is: https://github.com/carapace-finance/credit-default-swaps-contracts/pull/60

[clems4ev3r]

Fix looks good