search

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

0 stars 0 forks source link

legitimate users may lose their redemption opportunities, and the redemption process becomes unfair and biased in favor of attackers. #231

Closed c4-bot-9 closed 4 months ago

c4-bot-9 commented 5 months ago

Lines of code

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

Vulnerability details

proposeRedemption, allows users to submit a proposal for redeeming their dUSD for collateral from ShortRecords. The function takes the following inputs:

RedemptionFacet.sol#proposeRedemption

The proposeRedemption function is expected to allow users to propose redemptions for ShortRecords with a CR below the maximum redemption CR. The proposals should be processed in a fair and unbiased manner, based on factors like the CR, redemption amount, and time of submission.

function proposeRedemption(
    address asset,
    MTypes.ProposalInput[] calldata proposalInput,
    uint88 redemptionAmount,
    uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
    // Function body
}

Lack of front-running protection in the proposeRedemption function. An attacker can monitor the mempool for incoming redemption proposals and quickly submit their own proposal with a lower CR, effectively front-running the legitimate proposal.

The issue lies in the fact that the function processes the redemption proposals in the order they are received, without any considerations for the CR or the time of submission. This allows an attacker to manipulate the redemption order and potentially steal the redemption opportunity from legitimate users.

Impact

The redemption process becomes unfair and biased, favoring attackers over legitimate users.

Proof of Concept

Let's consider a scenario where multiple users want to propose redemptions for ShortRecords with different CRs. The ShortRecords and their corresponding CRs are as follows:

Legitimate User's Actions:

  1. A legitimate user, Alice, wants to propose a redemption for ShortRecord 1, which has a CR of 1.5.
  2. Alice calls the proposeRedemption function with the following parameters:
    • asset: The address of the asset being redeemed.
    • proposalInput: An array containing the shorter address, shortId, and shortOrderId of ShortRecord 1.
    • redemptionAmount: The amount of dUSD Alice wants to redeem.
    • maxRedemptionFee: The maximum fee Alice is willing to pay.
  3. Alice's transaction is broadcasted to the Ethereum network and enters the mempool.

Attacker's Actions:

  1. An attacker, Eve, is monitoring the mempool for redemption proposals.
  2. Eve sees Alice's transaction proposing a redemption for ShortRecord 1 with a CR of 1.5.
  3. Eve quickly creates her own redemption proposal for ShortRecord 2, which has a CR of 1.6.
  4. Eve sets a higher gas price to ensure that her transaction is prioritized by miners and included in the next block before Alice's transaction.
  5. Eve calls the proposeRedemption function with the following parameters:
    • asset: The same asset address as Alice's proposal.
    • proposalInput: An array containing the shorter address, shortId, and shortOrderId of ShortRecord 2.
    • redemptionAmount: The same or higher amount of dUSD compared to Alice's proposal.
    • maxRedemptionFee: A higher maximum fee than Alice's proposal to make it more attractive for miners.
  6. Eve's transaction is mined and included in the next block, effectively front-running Alice's proposal.

As can be seen:

  1. Eve's redemption proposal for ShortRecord 2 is processed before Alice's proposal for ShortRecord 1, even though ShortRecord 1 has a lower CR.
  2. If the redemption amount proposed by Eve is sufficient to fully redeem ShortRecord 2, Alice's proposal for ShortRecord 1 may be invalidated or left with a smaller redemption amount.
  3. Eve successfully manipulates the redemption order and potentially steals the redemption opportunity from Alice.
  4. Alice's redemption proposal may be partially or fully unfulfilled, causing her to lose out on the desired redemption.
  5. The front-running attack undermines the fairness and integrity of the redemption process, as it favors attackers who can manipulate the transaction ordering.
  6. Users may lose confidence in the DittoETH protocol if they consistently face front-running attacks and are unable to redeem their dUSD as intended.

This Code Snippet demonstrates the front-running

function proposeRedemption(
    address asset,
    MTypes.ProposalInput[] calldata proposalInput,
    uint88 redemptionAmount,
    uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
    // Function body

    for (uint256 i = 0; i < proposalInput.length; i++) {
        MTypes.ProposalInput memory proposal = proposalInput[i];
        STypes.ShortRecord storage shortRecord = s.shortRecords[asset][proposal.shorter][proposal.shortId];

        // Calculate CR and check if it's below the maximum redemption CR
        uint256 cr = calculateCR(shortRecord);
        if (cr >= MAX_REDEMPTION_CR) {
            continue;
        }

        // Update state variables and emit event
        // ...
    }
}

The proposeRedemption function iterates through the proposalInput array and processes each proposal in the order they are received. The lack of front-running protection allows an attacker to manipulate the transaction ordering and front-run legitimate proposals.

Tools Used

Manual Review

Recommended Mitigation Steps

Should implement measures such as a commit-reveal scheme or a priority queue based on factors like CR, redemption amount, and time of submission.

struct RedemptionProposal {
    address proposer;
    uint256 cr;
    uint256 redemptionAmount;
    uint256 submissionTime;
}

RedemptionProposal[] public redemptionQueue;

function proposeRedemption(
    address asset,
    MTypes.ProposalInput[] calldata proposalInput,
    uint88 redemptionAmount,
    uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
    // Function body

    RedemptionProposal memory proposal = RedemptionProposal({
        proposer: msg.sender,
        cr: calculateCR(proposalInput),
        redemptionAmount: redemptionAmount,
        submissionTime: block.timestamp
    });

    // Insert the proposal into the priority queue based on CR, redemption amount, and submission time
    insertProposalToPriorityQueue(proposal);
}

function processRedemptionProposals() external {
    // Process proposals from the priority queue
    while (redemptionQueue.length > 0) {
        RedemptionProposal memory proposal = redemptionQueue[0];
        // Process the proposal
        // ...
        // Remove the processed proposal from the queue
        removeProposalFromPriorityQueue(0);
    }
}

Redemption proposals are inserted into a priority queue based on their CR, redemption amount, and submission time. The processRedemptionProposals function then processes the proposals in the order determined by the priority queue, ensuring a fair and unbiased redemption process.

Assessed type

Context

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as duplicate of #206

c4-pre-sort commented 5 months ago

raymondfam marked the issue as not a duplicate

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

The described POC isn't as discrete as it should be in #35. Additionally, issues related to front-running are known issues per readme.

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Out of scope