sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

__141345__ - Should Accrue Premium at the beginning of protection #280

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

141345

medium

Should Accrue Premium at the beginning of protection

Summary

The accruing of premium is after the expiry of protection, this mechanism could lead to buyers' loss when the payout amount is low. And also could lead to inconsistent payout amounts when the lending default at different time. If accruing premium at the beginning of the protection, buyers will be better protected and the protection logic and mechanism will be more consistent.

Vulnerability Detail

Currently, accruing of premium only happens after the protection is expired.

File: contracts/core/pool/ProtectionPool.sol
0963:   function _accruePremiumAndExpireProtections() {

0985:       /// Verify & accrue premium for the protection and
0986:       /// if the protection is expired, then mark it as expired
0987:       (
0988:         uint256 _accruedPremiumInUnderlying,
0989:         bool _expired
0990:       ) = ProtectionPoolHelper.verifyAndAccruePremium(
0991:           poolInfo,
0992:           protectionInfo,
0993:           _lastPremiumAccrualTimestamp,
0994:           _latestPaymentTimestamp
0995:         );
0996:       _accruedPremiumForLendingPool += _accruedPremiumInUnderlying;
0997: 
0998:       if (_expired) {
0999:         /// Add removed protection amount to the total protection removed
1000:         _totalProtectionRemoved += protectionInfo
1001:           .purchaseParams
1002:           .protectionAmount;

File: contracts/libraries/ProtectionPoolHelper.sol
201:   function verifyAndAccruePremium(

228:     /// Only accrue premium if the protection is expired
229:     /// or latest payment is made after the protection start & last premium accrual
230:     if (
231:       _protectionExpired ||
232:       (_latestPaymentTimestamp > _startTimestamp &&
233:         _latestPaymentTimestamp > _lastPremiumAccrualTimestamp)
234:     ) {

And the accrued amount will be added to totalSTokenUnderlying.

279:   function accruePremiumAndExpireProtections(address[] memory _lendingPools)

317:       /// Iterate all active protections for this lending pool and
318:       /// accrue premium and expire protections if there is new payment
319:       (
320:         uint256 _accruedPremiumForLendingPool,
321:         uint256 _totalProtectionRemovedForLendingPool
322:       ) = _accruePremiumAndExpireProtections(
323:           lendingPoolDetail,
324:           _lastPremiumAccrualTimestamp,
325:           _latestPaymentTimestamp
326:         );
327:       _totalPremiumAccrued += _accruedPremiumForLendingPool;

341:     }

344:     if (_totalPremiumAccrued > 0) {
345:       totalPremiumAccrued += _totalPremiumAccrued;
346:       totalSTokenUnderlying += _totalPremiumAccrued;
347:     }

totalSTokenUnderlying is used as the source of payout for defaulted lending pools.

357:   function lockCapital(address _lendingPoolAddress)

415:       if (totalSTokenUnderlying < _lockedAmount) {
416:         /// If totalSTokenUnderlying < _lockedAmount, then lock all available capital
417:         _lockedAmount = totalSTokenUnderlying;
418:         totalSTokenUnderlying = 0;
419:       } else {
420:         /// Reduce the total sToken underlying amount by the locked amount
421:         totalSTokenUnderlying -= _lockedAmount;
422:       }

However, the deferral of accruing premium will make the total available payout amount smaller. The issue could arise when the payout fund is not enough. The un-accrued premium could have been used to cover buyers' loss, but the accounting just not yet include this part.

Imagine, some lending pool have late payment since day 1 of the protection, when the status changed to Defaulted, the protection might not expire. So the premium will not be used to cover this default. And if at the same time the protection pool has low balance of underlying, the buyers could suffer more loss.

Accruing after expiration will also introduce inconsistency for the payout mechanism. Continue with the above example, if the late payment occurred at the last day of the protection, when the status changed to Defaulted, the premium is included in the totalSTokenUnderlying for payout. The payout amounts will be higher, the difference is the premium for this specific premium.

Impact

The buyers could suffer some loss due to insufficient fund of payout. The protection will give inconsistent results with different lending pool defaults time.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Accrue Premium at the beginning of the the protection, and this amount can be used towards this protection for sure. And the protection payout results will be consistent with different time of defaulted lending.

vnadoda commented 1 year ago

@clems4ev3r This is an invalid concern. Premium accrues not after expiration but every time there is a payment to the lending pool. We are also planning to change that to continuous accrual as discussed in #294. Also, accruing premium on each buyProtection will be expensive as discussed in #288

clems4ev3r commented 1 year ago

@vnadoda, agreed this is invalid

hrishibhat commented 1 year ago

Closing based on the above comments