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

if TAPP.ethEscrowed > vault.ethEscrowed , updateYield() will revert

llill

Summary

DittoEth incentivizes short creation by adding yield to these position, yield for positions is updated with the function updateYield() in LibVault.sol.
However if for any reason the TAPP has more ethEscrowed than the vault, this function will revert

Vulnerability Details

LibVault.sol#updateYield() is called via YieldFacet.sol#updateYield() :

function updateYield(uint256 vault) external nonReentrant {
//E in LibVault => update yield rate of a vault from staking reward earned by vault bridges , decrement tithe and zethTreasuryReward from it
//E increment TAPP.ethEscrowed ,vault.zethYieldRate and vault.zethCollateralReward
vault.updateYield();
emit Events.UpdateYield(vault);
}

LibVault.sol#UpdateYield() calculates the yield by taking how much zeth has been added to the vault , if no zeth has been added it return, if new zeth has been added yield is calculated by calculating the difference between old balance of zeth and new balance :

uint88 yield = zethTotalNew - zethTotal;

Then yield is decremented by calculating how much the treasury (TAPP) owns zeth by dividing this amount from the total of zeth owned by the vault.

uint88 zethTreasury = TAPP.ethEscrowed;
uint88 zethTotal = Vault.zethTotal;
uint88 yield = zethTotalNew - zethTotal;
uint88 zethTreasuryReward = yield.mul(zethTreasury).divU88(zethTotal);

However the total number of zeth owned by the vault is not an addition of how much users deposited + how much the treasury owns because treasury can own zeth through multiple process like liquidation :

// MarginCallSecondaryFacet :
if (m.cRatio > 1 ether) {
//E calculate debt value in Ether using latest oracle Price
uint88 ercDebtAtOraclePrice = m.short.ercDebt.mulU88(LibOracle.getPrice(m.asset)); // eth
//E increase collateral that liquidator receives for this liquidation with this updated amount
m.liquidatorCollateral = ercDebtAtOraclePrice;
​
// if cRatio > 110%, liquidated user gets remaining collateral Otherwise they take a penalty, and remaining goes to the pool
address remainingCollateralAddress = m.cRatio > m.minimumCR
? m.shorter
: address(this);
//E update remaining collateral using remainingCollateralAddress
​
s.vaultUser[m.vault][remainingCollateralAddress].ethEscrowed += m.short.collateral - ercDebtAtOraclePrice;
}
​
// MarginCallPrimaryFacet :
function _marginFeeHandler(MTypes.MarginCallPrimary memory m) private {
//E fetch health struct of liquidator
STypes.VaultUser storage VaultUser = s.vaultUser[m.vault][msg.sender];
//E fetch health struct of address(this)
STypes.VaultUser storage TAPP = s.vaultUser[m.vault][address(this)];
// distribute fees to TAPP and caller for liquidation
uint88 tappFee = m.ethFilled.mulU88(m.tappFeePct);
uint88 callerFee = m.ethFilled.mulU88(m.callerFeePct) + m.gasFee;
​
m.totalFee += tappFee + callerFee;
//E TAPP already received the gasFee for being the forcedBid caller. tappFee nets out.
if (TAPP.ethEscrowed >= callerFee) {
TAPP.ethEscrowed -= callerFee;
VaultUser.ethEscrowed += callerFee;
} else {
//E Give caller (portion of?) tappFee instead of gasFee
VaultUser.ethEscrowed += callerFee - m.gasFee + tappFee;
m.totalFee -= m.gasFee;
TAPP.ethEscrowed -= m.totalFee;
}
}

and treasury can own zeth through withdraw :

function withdraw(address bridge, uint88 zethAmount)
external
nonReentrant
onlyValidBridge(bridge)
{
if (zethAmount == 0) revert Errors.ParameterIsZero();
​
uint88 fee;
uint256 withdrawalFee = bridge.withdrawalFee();
uint256 vault;
if (bridge == rethBridge || bridge == stethBridge) {
vault = Vault.CARBON;
} else {
vault = s.bridge[bridge].vault;
}
​
if (withdrawalFee > 0) {
fee = zethAmount.mulU88(withdrawalFee);
zethAmount -= fee;
s.vaultUser[vault][address(this)].ethEscrowed += fee; //E @audit TAPP incremented
}
vault.removeZeth(zethAmount, fee); //E vault.ethEscrowed -= amount
}

So as you can see tapp.ethEscrowed can become bigger as vault.ethEscrowed can be decremented in case of withdrawal.

The main point of this finding is that if tapp.ethEscrowed become bigger than vault.zethTotal it is a problem for updateYield() function. The problem is here :

uint88 zethTreasury = TAPP.ethEscrowed;
uint88 zethTotal = Vault.zethTotal;
uint88 yield = zethTotalNew - zethTotal;
uint88 zethTreasuryReward = yield.mul(zethTreasury).divU88(zethTotal);
yield -= zethTreasuryReward;

here we can see that if yield < zethTreasuryReward it will revert with an underflow error

=> if yield < yield.mul(zethTreasury).divU88(zethTotal)

=> if zethTotal < zethTreasury it will revert

So if TAPP.ethEscrowed > Vault.zethTotal, libVault.updateYield() will revert and yield won't be able to be distributed anymore using YieldFacet.sol

Impact

DOS of yieldFacet.sol#updateYield()

Tools Used

VSCode

Recommendations

One method could be to add a check to ensure zethTotal > zethTreasury and take the fee only if this check pass, otherwise give the full yield to the vault , or simply revert if zethTotal > zethTreasury

Updates

Lead Judging Commences

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

Support

FAQs

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