ck - `ReferenceLendingPools::_getLendingPoolStatus` will detect paused Goldfinch pools as active

[github-actions[bot]]

ck

medium

ReferenceLendingPools::_getLendingPoolStatus will detect paused Goldfinch pools as active

Summary

ReferenceLendingPools::_getLendingPoolStatus will detect paused Goldfinch pools as active

Vulnerability Detail

ReferenceLendingPools::_getLendingPoolStatus will return LendingPoolStatus.Active if the other conditions being checked fail:

function _getLendingPoolStatus(address _lendingPoolAddress)
    internal
    view
    returns (LendingPoolStatus)
  {
    if (!_isReferenceLendingPoolAdded(_lendingPoolAddress)) {
      return LendingPoolStatus.NotSupported;
    }

    ILendingProtocolAdapter _adapter = _getLendingProtocolAdapter(
      _lendingPoolAddress
    );

    if (_adapter.isLendingPoolExpired(_lendingPoolAddress)) {
      return LendingPoolStatus.Expired;
    }

    if (
      _adapter.isLendingPoolLateWithinGracePeriod(
        _lendingPoolAddress,
        Constants.LATE_PAYMENT_GRACE_PERIOD_IN_DAYS
      )
    ) {
      return LendingPoolStatus.LateWithinGracePeriod;
    }

    if (_adapter.isLendingPoolLate(_lendingPoolAddress)) {
      return LendingPoolStatus.Late;
    }

    return LendingPoolStatus.Active;
  }

One important condition that is not checked is whether the pool is paused which is a state GoldFinch pools can be in. If a pool is paused, _getLendingPoolStatus will instead return LendingPoolStatus.Active.

Impact

GoldFinch may pause pools for critical reasons e.g an exploit but _getLendingPoolStatus would still return it as active which could have various implications e.g sellers could sell protection for a paused pool.

Code Snippet

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

https://github.com/goldfinch-eng/mono/blob/bd9adae6fbd810d1ebb5f7ef22df5bb6f1eaee3b/packages/client2/lib/pools/index.ts#L50-L68

Tool used

Manual Review

Recommendation

Also check for the paused condition in GoldFinch pools - https://etherscan.io/address/0x4Df1e7fFB382F79736CA565F378F783678d995D8#readContract

Duplicate of #262

[iamckn]

Escalate for 25 USDC

The arguments against this being an issue are that if GoldFinch pauses a pool for long periods, it would be equivalent to a default. This ignores the issue decribed here where the state of a paused pool is marked incorrectly as Active by ReferenceLendingPools::_getLendingPoolStatus.

The other argument that GoldFinch is unlikely to keep a pool paused for a long period is making a dangerous assumption. A pool could be paused for various reasons such as an exploit that needs time to resolve. In the meantime, Carapace would still continue considering that pool as active as described in my issue. Among other implications is that protection sellers would essentially be depositing protection for a pool undergoing an exploit while incorrectly assuming that the pool is active as per the ReferenceLendingPools::_getLendingPoolStatus return value of Active.

[sherlock-admin]

Escalate for 25 USDC

The arguments against this being an issue are that if GoldFinch pauses a pool for long periods, it would be equivalent to a default. This ignores the issue decribed here where the state of a paused pool is marked incorrectly as Active by ReferenceLendingPools::_getLendingPoolStatus.

The other argument that GoldFinch is unlikely to keep a pool paused for a long period is making a dangerous assumption. A pool could be paused for various reasons such as an exploit that needs time to resolve. In the meantime, Carapace would still continue considering that pool as active as described in my issue. Among other implications is that protection sellers would essentially be depositing protection for a pool undergoing an exploit while incorrectly assuming that the pool is active as per the ReferenceLendingPools::_getLendingPoolStatus return value of Active.

You've created a valid escalation for 25 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.

[Evert0x]

Escalation accepted, issue is already labeled non-reward

[sherlock-admin]

Escalation accepted, issue is already labeled non-reward

This issue's escalations have been accepted!

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