Risky debt accumulates, threatens system stability, solvency. || Timely liquidation hindered, unhealthy positions persist, system risks.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L47-L90

Vulnerability details

Impact

• If minShortErc is set too low:
  • The system may accumulate a significant amount of risky debt that is not being liquidated due to a lack of liquidator incentives.
  • This can pose a threat to the stability and solvency of the system, as the accumulated risky debt may become unmanageable.
• If minShortErc is set too high:
  • The system may fail to liquidate risky ShortRecords in a timely manner, allowing unhealthy positions to persist and potentially worsen over time.
  • This can lead to a deterioration of the overall health of the system and increase the likelihood of a catastrophic failure.

Description

• The minShortErc parameter is used in the PrimaryLiquidationFacet.sol contract's liquidate function to ensure that a ShortRecord has a minimum amount of ercDebt to be eligible for liquidation.

• https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L47-L90

   function liquidate(address asset, address shorter, uint8 id, uint16[] memory shortHintArray)
       external
       isNotFrozen(asset)
       nonReentrant
       onlyValidShortRecord(asset, shorter, id)
       returns (uint88, uint88)
   {
       // ...
       LibSRUtil.checkCancelShortOrder({
           asset: asset,
           initialStatus: s.shortRecords[asset][shorter][id].status,
           shortOrderId: shortOrderId,
           shortRecordId: id,
           shorter: shorter
       });
       // ...
   }

• The LibSRUtil.checkCancelShortOrder is called to ensure that the ShortRecord's ercDebt is above minShortErc and to cancel the associated short order if necessary.

Vulnerability Details

• When the minShortErc parameter is not set appropriately.
• If minShortErc is set too low:
  • Liquidators may not have sufficient incentive to liquidate risky debt because the potential caller fee they earn (based on the liquidated collateral) may not cover the costs and effort involved in the liquidation process.
  • This can lead to a situation where risky ShortRecords with low ercDebt remain unliquidated, allowing the debt to accumulate and potentially threatening the stability of the system.
• If minShortErc is set too high:
  • Legitimate ShortRecords with ercDebt below the high threshold may not be eligible for liquidation, even if they pose a risk to the system.
  • This can prevent timely liquidation of risky debt and lead to a buildup of unhealthy positions in the system.

• When minShortErc is set too low, liquidators may not be motivated to liquidate ShortRecords with low ercDebt due to insufficient incentives. This can result in a concentration of risky debt that is not being adequately managed.
• When minShortErc is set too high, ShortRecords with ercDebt below the high threshold may not be eligible for liquidation, even if they are risky. This can hinder the timely liquidation of unhealthy positions and lead to a buildup of risky debt in the system.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider introducing additional incentives or mechanisms to encourage liquidators to actively manage risky positions, even if the ercDebt is relatively low.

• The minShortErc parameter could be dynamically adjusted based on system conditions:

   function adjustMinShortErc(uint256 newMinShortErc) external onlyOwner {
       require(newMinShortErc > 0, "Invalid minShortErc value");
       uint256 oldMinShortErc = minShortErc;
       minShortErc = newMinShortErc;
       emit MinShortErcUpdated(oldMinShortErc, newMinShortErc);
   }

  the adjustMinShortErc allows the contract owner to dynamically update the minShortErc parameter based on changing market conditions or system requirements.

   function liquidate(address asset, address shorter, uint8 id, uint16[] memory shortHintArray)
       external
       isNotFrozen(asset)
       nonReentrant
       onlyValidShortRecord(asset, shorter, id)
       returns (uint88, uint88)
   {
       // ...
       LibSRUtil.checkCancelShortOrder({
           asset: asset,
           initialStatus: s.shortRecords[asset][shorter][id].status,
           shortOrderId: shortOrderId,
           shortRecordId: id,
           shorter: shorter
       });
       // ...
   }

• The LibSRUtil.checkCancelShortOrder function is responsible for ensuring that the ShortRecord's ercDebt is above minShortErc:

   function checkCancelShortOrder(address asset, SR initialStatus, uint16 shortOrderId, uint8 shortRecordId, address shorter)
       internal
       returns (bool isCancelled)
   {
       // ...
       if (shorter == msg.sender) {
           // If call comes from exitShort() or combineShorts() then always cancel
           LibOrders.cancelShort(asset, shortOrderId);
           assert(shortRecord.status != SR.PartialFill);
           return true;
       } else if (shortRecord.ercDebt < LibAsset.minShortErc(asset)) {
           // If call comes from liquidate() and SR ercDebt under minShortErc
           LibOrders.cancelShort(asset, shortOrderId);
           return true;
       }
   }

By carefully setting and managing the minShortErc parameter, the system can ensure that liquidators are properly incentivized to liquidate risky debt and that the overall health and stability of the system are maintained.

Assessed type

Governance

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #261

[raymondfam]

System parameters are settable by owner, and may change before launch.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof