DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Incorrect condition in the MarginCallPrimaryFacet#`_performForcedBid()`

0xmuxyz

Summary

Within the MarginCallPrimaryFacet#_performForcedBid() above, in the case of m.cRatio == m.minimumCR, false is supposed to be stored into the m.loseCollateral. Because in the case of m.cRatio == m.minimumCR, the maintain CR (m.cRatio) does not below the minimum CR (m.minimumCR).

However, within the MarginCallPrimaryFacet#_performForcedBid() above, even when m.cRatio == m.minimumCR, true would be stored into the m.loseCollateral, meaning the Shorter loses leftover collateral despite their CR does not below the minimum CR (m.minimumCR).

Vulnerability Details

Within the MTypes library of the DataTypes contract, the MarginCallPrimary struct would be defined. And the cRatio and the minimumCR and the loseCollateral would be defined as a property of it like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/libraries/DataTypes.sol#L221
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/libraries/DataTypes.sol#L227
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/libraries/DataTypes.sol#L232

struct MarginCallPrimary {
address asset;
uint256 vault;
STypes.ShortRecord short;
address shorter;
uint256 cRatio; ///<---------- @audit
uint80 oraclePrice;
uint256 forcedBidPriceBuffer;
uint256 ethDebt;
uint88 ethFilled;
uint88 ercDebtMatched;
bool loseCollateral; ///<---------- @audit
uint256 tappFeePct;
uint256 callerFeePct;
uint88 gasFee;
uint88 totalFee; // gasFee + tappFee + callerFee
uint256 minimumCR; ///<---------- @audit
}

Within the MarginCallPrimaryFacet#_performForcedBid(), the true would be stored into the m.loseCollateral if maintain CR (m.cRatio) above minimum CR (m.minimumCR), meaning a Shorter loses leftover collateral like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/facets/MarginCallPrimaryFacet.sol#L213

/**
* @notice Handles the set up and execution of making a forcedBid
* @dev Shorter will bear the cost of forcedBid on market
* @dev Depending on shorter's cRatio, the TAPP can attempt to fund bid
*
* @param m Memory struct used throughout MarginCallPrimaryFacet.sol
* @param shortHintArray Array of hintId for the id to start matching against shorts since you can't match a short < oracle price
*
*/
​
function _performForcedBid(
MTypes.MarginCallPrimary memory m,
uint16[] memory shortHintArray
) private {
uint256 startGas = gasleft();
uint88 ercAmountLeft;
​
//@dev Provide higher price to better ensure it can fully fill the margin call
uint80 _bidPrice = m.oraclePrice.mulU80(m.forcedBidPriceBuffer);
​
// Shorter loses leftover collateral to TAPP when unable to maintain CR above the minimum
m.loseCollateral = m.cRatio <= m.minimumCR; ///<------------- @audit
...

Within the MarginCallPrimaryFacet#_performForcedBid() above, in the case of m.cRatio == m.minimumCR, false is supposed to be stored into the m.loseCollateral. Because in the case of m.cRatio == m.minimumCR, the maintain CR (m.cRatio) does not below the minimum CR (m.minimumCR).

However, within the MarginCallPrimaryFacet#_performForcedBid() above, even when m.cRatio == m.minimumCR, true would be stored into the m.loseCollateral, meaning the Shorter loses leftover collateral despite their CR does not below the minimum CR (m.minimumCR).

Impact

A Shorter loses leftover collateral despite their CR does not below the minimum CR (m.minimumCR).

Tools Used

  • Foundry

Recommendations

Within the MarginCallPrimaryFacet#_performForcedBid(), consider modify the condition like this:

function _performForcedBid(
MTypes.MarginCallPrimary memory m,
uint16[] memory shortHintArray
) private {
uint256 startGas = gasleft();
uint88 ercAmountLeft;
​
//@dev Provide higher price to better ensure it can fully fill the margin call
uint80 _bidPrice = m.oraclePrice.mulU80(m.forcedBidPriceBuffer);
​
// Shorter loses leftover collateral to TAPP when unable to maintain CR above the minimum
+ m.loseCollateral = m.cRatio < m.minimumCR;
- m.loseCollateral = m.cRatio <= m.minimumCR;
...
Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-171

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.