When calling function lockCapital(), the expiration status of the protection might not be up to date. It is possible that the protection just expired, but soon later, the lending pool status changed to Late, before the expired status of the protection come into effect. As a result, some seller could lose fund due to the expired protection.
The reason is, the updates and maintenance of the status is triggered manually by hand or by bot, not built into the smart contracts, hence it's possible that there exist some mismatch of the periods, when the Late status takes place during the gap period, the seller might suffer undeserved loss.
Vulnerability Detail
When calling function lockCapital(), activeProtectionIndexes will be looped to lock the capital.
File: contracts/core/pool/ProtectionPool.sol
357: function lockCapital(address _lendingPoolAddress) {
373: /// Get indexes of active protection for a lending pool from the storage
374: EnumerableSetUpgradeable.UintSet
375: storage activeProtectionIndexes = lendingPoolDetail
376: .activeProtectionIndexes;
377:
378: /// Iterate all active protections and calculate total locked amount for this lending pool
379: /// 1. calculate remaining principal amount for each loan protection in the lending pool.
380: /// 2. for each loan protection, lockedAmt = min(protectionAmt, remainingPrincipal)
381: /// 3. total locked amount = sum of lockedAmt for all loan protections
382: uint256 _length = activeProtectionIndexes.length();
383: for (uint256 i; i < _length; ) {
But even if some protection has expired, activeProtectionIndexes will not be changed immediately, the array will only be updated when accruing premium, in
So, there could be some gap period that the expired protection is still in the activeProtectionIndexes array, and being iterated when _assessState() -> _moveFromActiveToLockedState() -> lockCapital(). Some fund could be locked for default payout.
File: contracts/core/DefaultStateManager.sol
289: function _assessState(ProtectionPoolState storage poolState) internal {
324: if (
325: (_previousStatus == LendingPoolStatus.Active ||
326: _previousStatus == LendingPoolStatus.LateWithinGracePeriod) &&
327: _currentStatus == LendingPoolStatus.Late
328: ) {
329: /// Update the current status of the lending pool to Late
330: /// and move the lending pool to the locked state
331: lendingPoolStateDetail.currentStatus = LendingPoolStatus.Late;
332: _moveFromActiveToLockedState(poolState, _lendingPool);
390: function _moveFromActiveToLockedState(
391: ProtectionPoolState storage poolState,
392: address _lendingPool
393: ) internal {
397: (uint256 _lockedCapital, uint256 _snapshotId) = _protectionPool.lockCapital(
398: _lendingPool
399: );
The consequence is, there exists some edge cases that the protection just expired, but not yet accrued premium, the expired status not timely reflected in the activeProtectionIndexes, hence the sellers would suffer lose of fund for the period they do not cover the risk for.
Impact
Some seller could lose fund to cover the late/default for expired protections and lose fund.
Add protections expiration check in function _assessState(), record the timestamps of LendingPoolStatus changes. And based on the change timestamps to determine whether to lock capital, and ruling out the expired protections.
141345
medium
Expired protection capital could still be locked
Summary
When calling function
lockCapital(), the expiration status of the protection might not be up to date. It is possible that the protection just expired, but soon later, the lending pool status changed toLate, before the expired status of the protection come into effect. As a result, some seller could lose fund due to the expired protection.The reason is, the updates and maintenance of the status is triggered manually by hand or by bot, not built into the smart contracts, hence it's possible that there exist some mismatch of the periods, when the
Latestatus takes place during the gap period, the seller might suffer undeserved loss.Vulnerability Detail
When calling function
lockCapital(),activeProtectionIndexeswill be looped to lock the capital.But even if some protection has expired,
activeProtectionIndexeswill not be changed immediately, the array will only be updated when accruing premium, inSo, there could be some gap period that the expired protection is still in the
activeProtectionIndexesarray, and being iterated when_assessState()->_moveFromActiveToLockedState()->lockCapital(). Some fund could be locked for default payout.The consequence is, there exists some edge cases that the protection just expired, but not yet accrued premium, the expired status not timely reflected in the
activeProtectionIndexes, hence the sellers would suffer lose of fund for the period they do not cover the risk for.Impact
Some seller could lose fund to cover the late/default for expired protections and lose fund.
Code Snippet
https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L324-L332
https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L397-L399
https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ProtectionPool.sol#L373-L383
Tool used
Manual Review
Recommendation
Add protections expiration check in function
_assessState(), record the timestamps of LendingPoolStatus changes. And based on the change timestamps to determine whether to lock capital, and ruling out the expired protections.Duplicate of #305