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

0 stars 0 forks source link

Inconsistent redemption rate increase and fee calculation on redemption of undercollateralized assets #276

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

The redemption rate increases too much when redeeming undercollateralized short records. The redeemer pays too much (or inconsistent) fees.

Proof of Concept

When proposing a redemption the amount proposed is what is redeemed as collateral from a short record. When an undercollateralized short record is redeemed on the colRedeemed, accounting for how much collateral has been redeemed, it is capped to the short record's collateral balance. The amount proposed is not similarly reduced, however. RedemptionFacet.sol#L116-L125:

p.colRedeemed = p.oraclePrice.mulU88(p.amountProposed);
if (p.colRedeemed > currentSR.collateral) {
    p.colRedeemed = currentSR.collateral;
}

currentSR.collateral -= p.colRedeemed;
currentSR.ercDebt -= p.amountProposed;

p.totalAmountProposed += p.amountProposed;
p.totalColRedeemed += p.colRedeemed;

In this case p.totalColRedeemed is therefore smaller in value than p.totalAmountProposed.

These are the values used to calculate the redemption fee:

uint88 redemptionFee = calculateRedemptionFee(asset, p.totalColRedeemed, p.totalAmountProposed);

Specifically, the redemption fee is proportional to p.totalColRedeemed, but the increase in base rate is proportional to p.totalColRedeemed

Note how this effect can be used to assist an exploit described in my report titled "A successfully disputed redemption proposal has still increased the redemption fee base rate; exploit to depeg dUSD".

Recommended Mitigation Steps

If the collateral is insufficient to cover the proposed amount, reduce the proposed amount accordingly.

Assessed type

Other

c4-pre-sort commented 6 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 6 months ago

raymondfam marked the issue as primary issue

raymondfam commented 6 months ago

Opposing scenario to #205.

c4-sponsor commented 6 months ago

ditto-eth (sponsor) disputed

ditto-eth commented 6 months ago

From known issues: https://github.com/code-423n4/2024-03-dittoeth?tab=readme-ov-file#automated-findings--publicly-known-issues "Currently allowed to redeem at any CR under 2, even under 1 CR."

Instead of using dUSD to redeem at < 1 CR (and not get full value in ETH) user should just primary liquidate instead and get fees. In case of empty order book the user could make an ask to match with the forced bid in order to fulfill the liquidate

c4-judge commented 5 months ago

hansfriese marked the issue as unsatisfactory: Invalid