Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Malicious users can frontrun fee distribution to earn rewards

elolpuer

Summary

In VaultRouterBranch.sol:stake(), users stake their Vault.indexTokens to earn rewards based on their share. However malicious users can frontrun the reward distribution by quickly staking their tokens right before distribution, receiving the same proportional rewards as long-term stakers.

Vulnerability Details

Basically fee distribution being initiated by FeeConversionKeeper by checking checkUpkeep() function and then calling performUpkeep() to convert accumulated fees to WETH:

function checkUpkeep(bytes calldata /**/ ) external view returns (bool upkeepNeeded, bytes memory performData) {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
​
uint128[] memory liveMarketIds = self.marketMakingEngine.getLiveMarketIds();
​
bool distributionNeeded;
uint128[] memory marketIds = new uint128[]();
address[] memory assets = new address[]();
uint256 index = 0;
uint128 marketId;
​
// Iterate over markets by id
for (uint128 i; i < liveMarketIds.length; i++) {
marketId = liveMarketIds[i];
​
(address[] memory marketAssets, uint256[] memory feesCollected) =
self.marketMakingEngine.getReceivedMarketFees(marketId);
​
// Iterate over receivedMarketFees
for (uint128 j; j < marketAssets.length; j++) {
distributionNeeded = checkFeeDistributionNeeded(marketAssets[j], feesCollected[j]);
​
if (distributionNeeded) {
// set upkeepNeeded = true
upkeepNeeded = true;
​
// set marketId, asset
marketIds[index] = marketId;
assets[index] = marketAssets[j];
​
index++;
}
}
}
​
if (upkeepNeeded) {
performData = abi.encode(marketIds, assets);
}
}
​
// call FeeDistributionBranch::convertAccumulatedFeesToWeth
function performUpkeep(bytes calldata performData) external override onlyForwarder {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
​
IMarketMakingEngine marketMakingEngine = self.marketMakingEngine;
​
// decode performData
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
​
// convert accumulated fees to weth for decoded markets and assets
for (uint256 i; i < marketIds.length; i++) {
marketMakingEngine.convertAccumulatedFeesToWeth(marketIds[i], assets[i], self.dexSwapStrategyId, "");
}
}

In the marketMakingEngine.convertAccumulatedFeesToWeth() contract handles WETH reward distribution:

function _handleWethRewardDistribution(
Market.Data storage market,
address assetOut,
UD60x18 receivedWethX18
)
internal
{
// cache the total fee recipients shares as UD60x18
UD60x18 feeRecipientsSharesX18 = ud60x18(MarketMakingEngineConfiguration.load().totalFeeRecipientsShares);
​
// calculate the weth rewards for protocol and vaults
UD60x18 receivedProtocolWethRewardX18 = receivedWethX18.mul(feeRecipientsSharesX18);
UD60x18 receivedVaultsWethRewardX18 =
receivedWethX18.mul(ud60x18(Constants.MAX_SHARES).sub(feeRecipientsSharesX18));
​
// calculate leftover reward
UD60x18 leftover = receivedWethX18.sub(receivedProtocolWethRewardX18).sub(receivedVaultsWethRewardX18);
​
// add leftover reward to vault reward
receivedVaultsWethRewardX18 = receivedVaultsWethRewardX18.add(leftover);
​
// adds the weth received for protocol and vaults rewards using the assets previously paid by the engine
// as fees, and remove its balance from the market's `receivedMarketFees` map
market.receiveWethReward(assetOut, receivedProtocolWethRewardX18, receivedVaultsWethRewardX18);
​
// recalculate markes' vaults credit delegations after receiving fees to push reward distribution
Vault.recalculateVaultsCreditCapacity(market.getConnectedVaultsIds());
}

Due to the combination of market.receiveWethReward() and Vault.recalculateVaultsCreditCapacity(), the Distribution.distributeValue() function is triggered, resulting in fees being distributed to users.

function distributeValue(Data storage self, SD59x18 value) internal {
if (value.eq(SD59x18_ZERO)) {
return;
}
​
UD60x18 totalShares = ud60x18(self.totalShares);
​
if (totalShares.eq(UD60x18_ZERO)) {
revert Errors.EmptyDistribution();
}
​
SD59x18 deltaValuePerShare = value.div(totalShares.intoSD59x18());
​
self.valuePerShare = sd59x18(self.valuePerShare).add(deltaValuePerShare).intoInt256();
}

So by checking FeeConversionKeeper.sol:checkUpkeep() frontrunner can understand when Distribution.distributeValue() will be called.
And by staking his tokens just before fee distribution he can receive rewards.

PoC

Amounts to stake and distribute is minimum from test cases.

With frontrun:

  1. User stakes his tokens (amount = 101012) in contract

  2. Frontrunner sees that fee is being distributed to the contract. He stakes his tokens (amount = 101012)

  3. Fee being distributed with value equals 101012

  4. Now user claims his fee. Amount = 45455 (minus protocol weth reward)

  5. Frontrunner claims his fee. Amount = 45455 (minus protocol weth reward)

Without frontrun:

  1. User stakes his tokens (amount = 101012) in contract.

  2. Fee being distributed with value equals 101012

  3. Now user claims his fee. Amount = 90910 (minus protocol weth reward)

Long term user claims 50% less fee because of frontrun.

Impact

The distribution of the fee is incorrect and unfair, as real users who stake their tokens for a longer term to maintain the liquidity of the protocol receive the same proportional amount as frontrunners. The protocol ignores users' long-term commitment, calling its fairness into question.

Tools Used

Manual Review

Recommendations

Add a minimum amount of time in which tokens must be staked to be eligible for a reward.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Staking design is not fair for users who staked earlier and longer, frontrun fee distribution with big stake then unstake

Support

FAQs

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