code-423n4 / 2024-03-dittoeth-findings

0 stars 0 forks source link

Inflated `ercDebt` via manipulated `ercDebtRate` disrupts liquidation, causing congestion. Skewed liquidation efforts as liquidators target artificially inflated ShortRecords. #228

Closed c4-bot-10 closed 4 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L151-L163 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L156-L161

Vulnerability details

From the potential manipulation of the ercDebtRate parameter, which is stored in the Asset struct and can be updated by the protocol.

Impact

Proof of Concept

Manipulation of ercDebtRate to Inflate ercDebt and Trigger Unexpected Liquidations

Assume there are multiple ShortRecords in the system with varying levels of ercDebt, some of which are below the minShortErc threshold. An attacker wants to manipulate the ercDebtRate to inflate the ercDebt of these ShortRecords and make them eligible for liquidation.

Pace:

  1. The attacker creates a large number of ShortRecords with low ercDebt values, below the minShortErc threshold. For example:

    • ShortRecord A: ercDebt = 500 ether
    • ShortRecord B: ercDebt = 600 ether
    • ShortRecord C: ercDebt = 400 ether
    • ...
  2. The current ercDebtRate for the asset is set to a moderate value, let's say 1.2 (120%).

  3. The attacker manipulates the system to significantly increase the ercDebtRate for the asset. They manage to set the ercDebtRate to a much higher value, such as 10.0 (1000%).

  4. The updateErcDebt function is called for each ShortRecord, either through a system process or by the attacker themselves.

  5. Inside the updateErcDebt function, the ercDebt adjustment is calculated using the manipulated ercDebtRate: https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L156

    uint88 ercDebt = short.ercDebt.mulU88(ercDebtRate - short.ercDebtRate);

    For ShortRecord A:

    • ercDebt = 500 ether * (10.0 - 1.2) = 4,400 ether

    For ShortRecord B:

    • ercDebt = 600 ether * (10.0 - 1.2) = 5,280 ether

    For ShortRecord C:

    • ercDebt = 400 ether * (10.0 - 1.2) = 3,520 ether
  6. The calculated ercDebt adjustment is added to the existing ercDebt of each ShortRecord: https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L158-L161

    if (ercDebt > 0) {
       short.ercDebt += ercDebt;
       short.ercDebtRate = ercDebtRate;
    }

    For ShortRecord A:

    • ercDebt = 500 ether + 4,400 ether = 4,900 ether

    For ShortRecord B:

    • ercDebt = 600 ether + 5,280 ether = 5,880 ether

    For ShortRecord C:

    • ercDebt = 400 ether + 3,520 ether = 3,920 ether
  7. As a result of the ercDebtRate manipulation and the subsequent updateErcDebt calls, the ercDebt of the ShortRecords has drastically increased.

  8. Assuming the minShortErc threshold is set to 1,000 ether, the inflated ercDebt values now exceed this threshold, making the ShortRecords eligible for liquidation.

  9. Liquidators are now incentivized to liquidate these ShortRecords due to their high ercDebt values, even though they originally had low debt levels.

  10. The sudden influx of ShortRecords eligible for liquidation can overwhelm the liquidation process and create congestion in the system.

Hit:

  • The manipulation of the ercDebtRate allows an attacker to artificially inflate the ercDebt of ShortRecords, bypassing the intended debt levels and liquidation thresholds.
  • ShortRecords that were previously considered safe and not eligible for liquidation suddenly become targets for liquidation due to the inflated ercDebt.
  • Users who own these ShortRecords may face unexpected liquidations and potential financial losses.
  • The influx of artificially inflated ShortRecords eligible for liquidation can strain the liquidation process, leading to congestion and potential instability in the system.
  • The manipulation of ercDebtRate undermines the integrity of the debt calculation and distribution mechanism, eroding trust in the protocol.

Recommended Mitigation Steps

Implement strong access controls and validation mechanisms for updating the ercDebtRate. The protocol should enforce strict limits on the magnitude and frequency of ercDebtRate changes to prevent sudden and drastic increases that can destabilize the system.

Additionally, implementing rate-limiting mechanisms, circuit breakers, and multi-sig approvals for ercDebtRate updates can help mitigate the risk of manipulation by malicious actors.

     function updateErcDebt(STypes.ShortRecord storage short, address asset) internal {
         AppStorage storage s = appStorage();

         uint64 ercDebtRate = s.asset[asset].ercDebtRate;
         uint64 maxDebtRateIncrease = s.maxDebtRateIncrease; // Maximum allowed increase in ercDebtRate

         if (ercDebtRate > short.ercDebtRate + maxDebtRateIncrease) {
             // Limit the ercDebtRate increase to the maximum allowed value
             ercDebtRate = short.ercDebtRate + maxDebtRateIncrease;
         }

         uint88 ercDebt = short.ercDebt.mulU88(ercDebtRate - short.ercDebtRate);

         if (ercDebt > 0) {
             short.ercDebt += ercDebt;
             short.ercDebtRate = ercDebtRate;
         }
     }

A maxDebtRateIncrease parameter is introduced to limit the maximum allowed increase in ercDebtRate during each update. If the ercDebtRate exceeds the ShortRecord's stored ercDebtRate by more than maxDebtRateIncrease, it is capped to the maximum allowed value. This helps prevent sudden and drastic increases in ercDebt due to ercDebtRate manipulation.

    function updateErcDebt(STypes.ShortRecord storage short, address asset) internal {
        AppStorage storage s = appStorage();

        // Distribute ercDebt
        uint64 ercDebtRate = s.asset[asset].ercDebtRate;
        uint88 ercDebt = short.ercDebt.mulU88(ercDebtRate - short.ercDebtRate);

        if (ercDebt > 0) {
            short.ercDebt += ercDebt;
            short.ercDebtRate = ercDebtRate;
        }
    }
}
     function updateErcDebt(STypes.ShortRecord storage short, address asset) internal {
         AppStorage storage s = appStorage();

         uint64 ercDebtRate = s.asset[asset].ercDebtRate;
         uint64 maxDebtRateIncrease = s.maxDebtRateIncrease; // Maximum allowed increase in ercDebtRate

         if (ercDebtRate > short.ercDebtRate + maxDebtRateIncrease) {
             // Limit the ercDebtRate increase to the maximum allowed value
             ercDebtRate = short.ercDebtRate + maxDebtRateIncrease;
         }

         uint88 ercDebt = short.ercDebt.mulU88(ercDebtRate - short.ercDebtRate);

         if (ercDebt > 0) {
             short.ercDebt += ercDebt;
             short.ercDebtRate = ercDebtRate;
         }
     }

Assessed type

Invalid Validation

raymondfam commented 5 months ago

ercDebtRate is admin-controlled and should be trusted.

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Invalid