DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

The stale `zethYieldRate` would be applied for the calculation in the YieldFacet#`_distributeYield()` due to lack of calling the YieldFacet#`updateYield()`

0xmuxyz

Summary

The YieldFacet#updateYield() is supposed to be called when the YieldFacet#_distributeYield() would be called via the YieldFacet#distributeYield(). Because the zethYieldRate used in the YieldFacet#_distributeYield() is supposed to be the latest rate.
However, within both the YieldFacet#distributeYield() and the YieldFacet#_distributeYield(), the YieldFacet#updateYield() would not be called.

This lead to a bad situation that the stale zethYieldRate, which is not the latest zethYieldRate, would be applied for the calculation in the YieldFacet#_distributeYield().

Since the YieldFacet#_distributeYield() would called via the YieldFacet#distributeYield(), the zETH yield, which is calculated based on the stale zethYieldRate, would be distributed to the Shorter (msg.sender).

Vulnerability Details

When the YieldFacet#updateYield() would be called, the vault yield rate from staking rewards earned by bridge contracts holding LSD would be updated.

Within the YieldFacet#updateYield(), the LibVault#updateYield() would be called like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/facets/YieldFacet.sol#L42

/**
* @notice Updates the vault yield rate from staking rewards earned by bridge contracts holding LSD
* @dev Does not distribute yield to any individual owner of shortRecords
*
* @param vault The vault that will be impacted
*/
​
function updateYield(uint256 vault) external nonReentrant {
vault.updateYield(); ///<-------------- @audit
...
}

Within the LibVault#updateYield(), the zethYieldRate would be updated to the latest rate and it would be stored into the AppStorage.vault storage (Vault.zethYieldRate) like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/libraries/LibVault.sol#L92

/**
* @notice Updates the vault yield rate from staking rewards earned by bridge contracts holding LSD
* @dev Does not distribute yield to any individual owner of shortRecords
*
* @param vault The vault that will be impacted
*/
​
function updateYield(uint256 vault) internal {
AppStorage storage s = appStorage();
​
STypes.Vault storage Vault = s.vault[vault];
STypes.VaultUser storage TAPP = s.vaultUser[vault][address(this)];
// Retrieve vault variables
uint88 zethTotalNew = uint88(getZethTotal(vault)); // @dev(safe-cast)
uint88 zethTotal = Vault.zethTotal;
uint88 zethCollateral = Vault.zethCollateral;
uint88 zethTreasury = TAPP.ethEscrowed;
​
// Calculate vault yield and overwrite previous total
if (zethTotalNew <= zethTotal) return;
uint88 yield = zethTotalNew - zethTotal;
Vault.zethTotal = zethTotalNew;
​
// If no short records, yield goes to treasury
if (zethCollateral == 0) {
TAPP.ethEscrowed += yield;
return;
}
​
// Assign yield to zethTreasury
uint88 zethTreasuryReward = yield.mul(zethTreasury).divU88(zethTotal);
yield -= zethTreasuryReward;
// Assign tithe of the remaining yield to treasuryF
uint88 tithe = yield.mulU88(vault.zethTithePercent());
yield -= tithe;
// Realize assigned yields
TAPP.ethEscrowed += zethTreasuryReward + tithe;
Vault.zethYieldRate += yield.divU80(zethCollateral); ///<----------------- @audit
Vault.zethCollateralReward += yield;
}

Within the YieldFacet#distributeYield(), the YieldFacet#_distributeYield() would be called like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/facets/YieldFacet.sol#L58

/**
* @notice Updates the vault yield rate from staking rewards earned by bridge contracts holding LSD
* @dev Can only distribute yield in markets that are part of the same vault
*
* @param assets Array of markets to evaluate when distributing yield from caller's shortRecords
*/
​
function distributeYield(address[] calldata assets) external nonReentrant {
uint256 length = assets.length;
uint256 vault = s.asset[assets[0]].vault;
​
// distribute yield for the first order book
(uint88 yield, uint256 dittoYieldShares) = _distributeYield(assets[0]); ///<-------------- @audit
...

Within the YieldFacet#_distributeYield(), the zethYieldRate-stored in the AppStorage.vault storage(s.vault[vault].zethYieldRate) would be used like this:
https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/facets/YieldFacet.sol#L83

// Distributes yield earned from all of caller's shortRecords of this asset
function _distributeYield(address asset)
private
onlyValidAsset(asset)
returns (uint88 yield, uint256 dittoYieldShares)
{
uint256 vault = s.asset[asset].vault;
// Last updated zethYieldRate for this vault
uint80 zethYieldRate = s.vault[vault].zethYieldRate; ///<-------- @audit
// Protocol time
uint256 timestamp = LibOrders.getOffsetTimeHours();
// Last saved oracle price
uint256 oraclePrice = LibOracle.getPrice(asset);
// CR of shortRecord collateralized at initialMargin for this asset
uint256 initialCR = LibAsset.initialMargin(asset) + 1 ether;
// Retrieve first non-HEAD short
uint8 id = s.shortRecords[asset][msg.sender][Constants.HEAD].nextId;
// Loop through all shorter's shorts of this asset
while (true) {
// One short of one shorter in this market
STypes.ShortRecord storage short = s.shortRecords[asset][msg.sender][id];
// To prevent flash loans or loans where they want to deposit to claim yield immediately
bool isNotRecentlyModified =
timestamp - short.updatedAt > Constants.YIELD_DELAY_HOURS;
// Check for cancelled short
if (short.status != SR.Cancelled && isNotRecentlyModified) {
uint88 shortYield =
short.collateral.mulU88(zethYieldRate - short.zethYieldRate);
// Yield earned by this short
yield += shortYield;
// Update zethYieldRate for this short
short.zethYieldRate = zethYieldRate;
// Calculate CR to modify ditto rewards
uint256 cRatio = short.getCollateralRatioSpotPrice(oraclePrice);
if (cRatio <= initialCR) {
dittoYieldShares += shortYield;
} else {
// Reduce amount of yield credited for ditto rewards proportional to CR
dittoYieldShares += shortYield.mul(initialCR).div(cRatio);
}
}
...
}
}

Based on above, the YieldFacet#updateYield() is supposed to be called when the YieldFacet#_distributeYield() would be called via the YieldFacet#distributeYield(). Because the zethYieldRate used in the YieldFacet#_distributeYield() is supposed to be the latest rate.
However, within both the YieldFacet#distributeYield() and the YieldFacet#_distributeYield(), the YieldFacet#updateYield() would not be called.

This lead to a bad situation that the stale zethYieldRate, which is not the latest zethYieldRate, would be applied for the calculation in the YieldFacet#_distributeYield().

Since the YieldFacet#_distributeYield() would called via the YieldFacet#distributeYield(), the zETH yield, which is calculated based on the stale zethYieldRate, would be distributed to the Shorter (msg.sender).

Impact

This lead to a bad situation that the stale zethYieldRate, which is not the latest zethYieldRate, would be applied for the calculation in the YieldFacet#_distributeYield().

Since the YieldFacet#_distributeYield() would called via the YieldFacet#distributeYield(), the zETH yield, which is calculated based on the stale zethYieldRate, would be distributed to the Shorter (msg.sender).

Tools Used

  • Foundry

Recommendations

Within the YieldFacet#distributeYield(), consider calling the YieldFacet#updateYield() before the YieldFacet#_distributeYield() would be called like his:

function distributeYield(address[] calldata assets) external nonReentrant {
uint256 length = assets.length;
uint256 vault = s.asset[assets[0]].vault;
+ updateYield(vault);
​
// distribute yield for the first order book
(uint88 yield, uint256 dittoYieldShares) = _distributeYield(assets[0]);
...
Updates

Lead Judging Commences

0xnevi Lead Judge
about 2 years ago
0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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