sherlock-audit / 2023-02-carapace-judging

2 stars 0 forks source link

joestakey - `assessState()` does not check when a pool is in `Defaulted` state #323

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

joestakey

medium

assessState() does not check when a pool is in Defaulted state

Summary

assessState() will return Late for a Defaulted pool

Vulnerability Detail

As per the docs:

The following are primary functions of ReferenceLendingPools:
Asses the payment status of the lending pool, i.e Active, Late, Expired, Defaulted, etc

But looking at _getLendingPoolStatus(), it actually does not handle the Defaulted case, returning Late instead

File: contracts/core/pool/ReferenceLendingPools.sol
318:   function _getLendingPoolStatus(address _lendingPoolAddress)
319:     internal
320:     view
321:     returns (LendingPoolStatus)
322:   {
323:     if (!_isReferenceLendingPoolAdded(_lendingPoolAddress)) {
324:       return LendingPoolStatus.NotSupported;
325:     }
326: 
327:     ILendingProtocolAdapter _adapter = _getLendingProtocolAdapter(
328:       _lendingPoolAddress
329:     );
330: 
331:     if (_adapter.isLendingPoolExpired(_lendingPoolAddress)) {
332:       return LendingPoolStatus.Expired;
333:     }
334: 
335:     if (
336:       _adapter.isLendingPoolLateWithinGracePeriod(
337:         _lendingPoolAddress,
338:         Constants.LATE_PAYMENT_GRACE_PERIOD_IN_DAYS
339:       )
340:     ) {
341:       return LendingPoolStatus.LateWithinGracePeriod;
342:     }
343: 
344:     if (_adapter.isLendingPoolLate(_lendingPoolAddress)) {
345:       return LendingPoolStatus.Late;
346:     }
347: 
348:     return LendingPoolStatus.Active;
349:   }

Impact

assessState() is here a view function, but returning that a pool is Late while it is defaulted may lead to issues with composability with external contracts querying this function to assess whether to perform certain actions or not.

Code Snippet

https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/pool/ReferenceLendingPools.sol#L322-L348

Tool used

Manual Review

Recommendation

The function should handle the Defaulted state too.

vnadoda commented 1 year ago

@clems4ev3r this is not a valid concern. Carapace protocol has its own criteria for default which is managed/assessed in DefaultStateManager and it doesn't rely on supported lending protocol, at least for now. See: https://github.com/sherlock-audit/2023-02-carapace/blob/main/contracts/core/DefaultStateManager.sol#L359

clems4ev3r commented 1 year ago

@vnadoda, agreed Carapace can have different parameters than individual lending protocols

hrishibhat commented 1 year ago

Closing based on the above comments