DittoETH

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

Changes in `dittoShorterRate` affect retroactively to accrued Ditto yield shares

Summary

The calculation of the Ditto rewards earned by shorters does not take into account that the changes in the Ditto shorter rate will impact retroactively, inflating or deflating the new Ditto rewards of the users.

Vulnerability Details

YieldFacet.sol:distributeYield() calculates and credits ZETH and Ditto rewards earned from short records by msg.sender.
The distribution of the rewards is performed in the _claimYield() function:

125 // Credit ZETH and Ditto rewards earned from shortRecords from all markets
126 function _claimYield(uint256 vault, uint88 yield, uint256 dittoYieldShares) private {
127 STypes.Vault storage Vault = s.vault[vault];
128 STypes.VaultUser storage VaultUser = s.vaultUser[vault][msg.sender];
129 // Implicitly checks for a valid vault
130 if (yield <= 1) revert Errors.NoYield();
131 // Credit yield to ethEscrowed
132 VaultUser.ethEscrowed += yield;
133 // Ditto rewards earned for all shorters since inception
134 uint256 protocolTime = LibOrders.getOffsetTime();
135 uint256 dittoRewardShortersTotal = Vault.dittoShorterRate * protocolTime;
136 // Ditto reward proportion from this yield distribution
137 uint256 dittoYieldSharesTotal = Vault.zethCollateralReward;
138 uint256 dittoReward =
139 dittoYieldShares.mul(dittoRewardShortersTotal).div(dittoYieldSharesTotal);
140 // Credit ditto reward to user
141 if (dittoReward > type(uint80).max) revert Errors.InvalidAmount();
142 VaultUser.dittoReward += uint80(dittoReward);
143 }

Focusing on the Ditto rewards, we can see that the function receives the number of yield shares earned by the user (dittoYieldShares) and in line 138 calculates the Ditto reward by multiplying this amount by the total amount of rewards of the protocol (dittoRewardShortersTotal) and dividing it by the total amount of yield shares of the protocol (dittoYieldSharesTotal).

If we take a look in line 135 at how the dittoRewardShortersTotal is calculated, we can see that it is the product of the Ditto shorter rate and total time elapsed since the protocol deployment.

This last calculation is wrong, as it is assumed that the Ditto shorter rate is constant, but this parameter can be changed by the admin or the DAO. This means that the changes in the Ditto shorter rate will impact retroactively, inflating or deflating the new Ditto rewards of the users. Also, users that have yielded the same number of shares during the same period, will receive different rewards depending on whether they claim their rewards before or after the Ditto shorter rate change.

Proof of Concept

Add the following code snippet into test/Yield.t.sol and run forge test --mt testYieldRateChange.

function testYieldRateChange() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address[] memory assets = new address[](1);
assets[0] = asset;
fundLimitBid(DEFAULT_PRICE, 320000 ether, receiver);
fundLimitShort(DEFAULT_PRICE, 80000 ether, alice);
fundLimitShort(DEFAULT_PRICE, 80000 ether, bob);
generateYield();
skip(yieldEligibleTime);
// Alice and Bob have the same number of Ditto yield shares
assertEq(diamond.getDittoMatchedReward(vault, alice), diamond.getDittoMatchedReward(vault, alice));
// Alice's yield is distributed
vm.prank(alice);
diamond.distributeYield(assets);
// Ditto shorter rate is updated
vm.prank(owner);
diamond.setDittoShorterRate(vault, 2);
// Bob's yield is distributed
vm.prank(bob);
diamond.distributeYield(assets);
uint256 aliceDittoRewards = diamond.getDittoReward(vault, alice);
uint256 bobDittoRewards = diamond.getDittoReward(vault, bob);
// Bob receives more Ditto rewards than Alice, even both were entitled to the same amount
assertApproxEqAbs(aliceDittoRewards * 2, bobDittoRewards, 2);
}

Impact

Changes in the Ditto shorter rate will impact retroactively, inflating or deflating the new Ditto rewards of the users. Users might not be incentivized to claim their rewards, as they might receive more rewards if they wait for the Ditto shorter rate to change.

Tools Used

Manual review.

Recommendations

Create two new state variables that keep track of the timestamp of the last Ditto shorter rate update and the total Ditto rewards accrued at that time. Then the calculation of dittoRewardShortersTotal would be:

uint256 dittoRewardShortersTotal = lastSnapshotRewards + Vault.dittoShorterRate * (protocolTime - lastSnapshotTimestamp);
Updates

Lead Judging Commences

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

finding-466

Support

FAQs

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