Disputants manipulate ShortRecord, claim undue penalties, disrupt redemption process.

[c4-bot-1]

Lines of code

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

Vulnerability details

Vulnerability 1: Manipulated ShortRecord with Lower CR

The dispute mechanism in the redemption process is vulnerable to manipulation by disputants who provide a ShortRecord with a lower CR that may not necessarily be the lowest available CR.

Description

• disputeRedemption allows anyone to challenge a proposed redemption by providing a ShortRecord with a lower CR than the one being disputed.
• The function retrieves the decoded proposal data, checks the validity of the dispute ShortRecord, and compares its CR with the CR of the incorrect proposal specified by the incorrectIndex.
• If the dispute ShortRecord has a lower CR and its updatedAt timestamp is earlier than the proposal's timeProposed, the dispute is considered valid.

Vulnerability Details

• Lies in the assumption that the disputer will provide a ShortRecord with the lowest available CR.
• But, a malicious disputer can manipulate a ShortRecord to have a lower CR than the disputed proposal, even if it is not the lowest available CR in the system.

  • This manipulation can be achieved by artificially lowering the collateral or increasing the debt of a ShortRecord, making it appear to have a lower CR.

• RedemptionFacet.sol#259

   if (disputeCR < incorrectProposal.CR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed)

• The dispute mechanism is expected to allow disputants to provide a ShortRecord with the lowest available CR to challenge a proposed redemption.
• The purpose is to ensure that the redemption process progresses from the lowest CR to the highest CR and prevents any incorrect or out-of-order redemptions.

• A malicious disputer can provide a manipulated ShortRecord with a lower CR that is not necessarily the lowest available CR.
• This allows the disputer to successfully dispute a redemption proposal even if there are other ShortRecords with lower CRs that should have been considered first.
• As a result, the disputer can claim undue penalties and disrupt the integrity of the redemption process.

Impact

• It allows malicious disputants to game the system and claim undue penalties, potentially leading to financial losses for honest redeemers.
• The integrity of the redemption mechanism is compromised, as it may not always progress from the lowest CR to the highest CR as intended.

Vulnerability 2: Timestamp Manipulation

dispute mechanism in the redemption process relies on the timeProposed and updatedAt timestamps, which are based on the protocol's time (LibOrders.getOffsetTime()). If these timestamps can be manipulated, it could impact the integrity of the dispute process.

Description

• disputeRedemption uses the timeProposed timestamp of the redemption proposal and the updatedAt timestamp of the dispute ShortRecord to determine the validity of a dispute.
• The function compares these timestamps to ensure that the dispute ShortRecord's updatedAt is earlier than the proposal's timeProposed plus a buffer (C.DISPUTE_REDEMPTION_BUFFER).
• RedemptionFacet.sol#259

   if (disputeCR < incorrectProposal.CR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed) {
       // ...
   }

Vulnerability Details

• If the timeProposed and updatedAt timestamps manipulated by an attacker.

• These timestamps are based on the protocol's time, which is obtained from LibOrders.getOffsetTime().

• If there are any vulnerabilities or weaknesses in the implementation of getOffsetTime() or the underlying time source, an attacker could potentially manipulate the timestamps.

• RedemptionFacet.sol#259

   if (disputeCR < incorrectProposal.CR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed) {
       // ...
   }

• If the timestamps can be manipulated, an attacker could potentially bypass the timestamp comparison and successfully dispute a redemption proposal even if the dispute ShortRecord's updatedAt is later than the proposal's timeProposed.

• This would allow the attacker to game the dispute process and claim undue penalties.

Impact

• Attackers could exploit the vulnerability to manipulate timestamps and successfully dispute redemption proposals that should not be disputed.
  • This could lead to financial losses for honest redeemers and disrupt the proper functioning of the redemption process.

Proof of Concept

Manipulated ShortRecord in Redemption Dispute

Scenario: Assume there are three ShortRecords in the system with the following collateral ratios (CRs):

• ShortRecord A: CR = 1.5
• ShortRecord B: CR = 1.2
• ShortRecord C: CR = 1.8

A malicious disputer, Eve, wants to game the redemption dispute process to claim undue penalties.

Pace:

1. Alice proposes a redemption for ShortRecord A with CR = 1.5.

2. Eve creates a new ShortRecord, ShortRecord D, with a manipulated CR of 1.1.

   • Eve artificially lowers the collateral or increases the debt of ShortRecord D to achieve the manipulated CR.

3. Eve calls the disputeRedemption function, providing ShortRecord D as the dispute ShortRecord.

4. The disputeRedemption function compares the CR of ShortRecord D (1.1) with the CR of ShortRecord A (1.5) and finds that ShortRecord D has a lower CR.

5. The function also checks if the updatedAt timestamp of ShortRecord D is earlier than the timeProposed of Alice's redemption proposal.

   • Eve ensures that the updatedAt timestamp of ShortRecord D is set to a value that satisfies this condition.

6. Since the CR of ShortRecord D is lower than ShortRecord A and the timestamp condition is met, the dispute is considered valid.

7. The function removes Alice's redemption proposal and imposes a penalty on Alice, transferring the penalty amount to Eve.

8. Eve successfully games the system and claims undue penalties, even though ShortRecord B with CR = 1.2 should have been considered first for redemption.

• The integrity of the redemption process is compromised, as the redemption order is not strictly followed from the lowest CR to the highest CR.
• Alice, an honest redeemer, suffers financial losses due to the undue penalty imposed on her.
• Eve, the malicious disputer, gains financial benefits by gaming the system and claiming undue penalties.
• The fairness and reliability of the redemption mechanism are undermined, potentially leading to a loss of trust among users.

Code snippet demonstrating the disputeRedemption function:

function disputeRedemption(address asset, address redeemer, uint8 incorrectIndex, address disputeShorter, uint8 disputeShortId)
    external
{
    // ...

    STypes.ShortRecord storage disputeSR = s.shortRecords[asset][disputeShorter][disputeShortId];
    uint256 disputeCR = disputeSR.getCollateralRatio(redeemerAssetUser.oraclePrice);

    MTypes.ProposalData memory incorrectProposal = decodedProposalData[incorrectIndex];

    if (disputeCR < incorrectProposal.CR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed) {
        // Dispute is considered valid
        // Remove incorrect proposals and impose penalty on redeemer
        // ...
    }

    // ...
}

In the condition disputeCR < incorrectProposal.CR, which allows a manipulated ShortRecord with a lower CR to be used for disputing, even if it is not the lowest available CR in the system.

Tools Used

Manual Review

Recommended Mitigation Steps

Additional checks should be implemented to ensure that the dispute ShortRecord has the lowest CR among all ShortRecords in the system by iterating through all ShortRecords and comparing their CRs, as suggested in the mitigation section of the previous response.

     function disputeRedemption(address asset, address redeemer, uint8 incorrectIndex, address disputeShorter, uint8 disputeShortId)
         external
     {
         // ...

         uint256 lowestCR = type(uint256).max;
         for (uint256 i = 0; i < numShortRecords; i++) {
             STypes.ShortRecord storage currentSR = s.shortRecords[asset][shorters[i]][shortIds[i]];
             uint256 currentCR = currentSR.getCollateralRatio(redeemerAssetUser.oraclePrice);
             if (currentCR < lowestCR) {
                 lowestCR = currentCR;
             }
         }

         if (disputeCR == lowestCR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed) {
             // Valid dispute, proceed with the dispute logic
             // ...
         } else {
             revert Errors.InvalidRedemptionDispute();
         }

         // ...
     }

• The vulnerability is present in the disputeRedemption

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

   if (disputeCR < incorrectProposal.CR && disputeSR.updatedAt + C.DISPUTE_REDEMPTION_BUFFER <= redeemerAssetUser.timeProposed) {
       // ...
   }

Assessed type

Governance

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #206

[raymondfam]

See #206.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof