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

Unclaimed Rewards Are Lost During Re-Staking

io10

Summary

Users lose unclaimed rewards whenever they stake new tokens. Specifically, the Distribution::_updateLastValuePerShare called in Distribution::accumulateActor(actorId) and Distribution::setActorShares which are both called in VaultRouterBranch::stake at stake time resets the user’s reward checkpoint (Distribution::lastValuePerShare) to the current global Distribution::valuePerShare before the user claims prior rewards. This zeroes out any accrued but unclaimed WETH rewards.

Vulnerability Details

VaultRouterBranch::stake calls wethRewardDistribution.accumulateActor(actorId) before updating the user’s share balance.
As explained in the above summary, internally, Distribution::accumulateActor calls Distribution::_updateLastValuePerShare, which sets actor.lastValuePerShare = self.valuePerShare.

By resetting Distribution::lastValuePerShare to the current global Distribution::valuePerShare, any difference between the actor’s old lastValuePerShare and the current valuePerShare (i.e., pending rewards) is effectively discarded. When the user later attempts to call FeeDistributionBranch::claimFees(), they find no rewards left to claim.

Proof Of Code (POC)

​
function testFuzz\_restakingerasespreviousgeneratedfees(
uint128 assetsToDepositA,
uint256 vaultId1,
uint256 vaultId2,
uint256 marketId1,
uint128 marketFees,
uint256 adapterIndex
)
external
{
vm.stopPrank();
​
//c configure vaults and markets
VaultConfig memory fuzzVaultConfig1 = getFuzzVaultConfig(vaultId1);
vm.assume(fuzzVaultConfig1.asset != address(usdc));
PerpMarketCreditConfig memory fuzzMarketConfig1 = getFuzzPerpMarketCreditConfig(marketId1);
​
​
VaultConfig memory fuzzVaultConfig2 = getFuzzVaultConfig(vaultId2);
vm.assume(fuzzVaultConfig2.asset != address(usdc));
vm.assume(fuzzVaultConfig1.vaultId != fuzzVaultConfig2.vaultId);
​
uint256[] memory marketIds = new uint256[](1);
marketIds[0] = fuzzMarketConfig1.marketId;
​
uint256[] memory vaultIds = new uint256[](1);
vaultIds[0] = fuzzVaultConfig1.vaultId;
​
vm.prank(users.owner.account);
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
​
​
//c deposit asset into vault
address userA = users.naruto.account;
uint128 assetsToDepositA =
uint128(bound(assetsToDepositA, calculateMinOfSharesToStake(fuzzVaultConfig1.vaultId), fuzzVaultConfig1.depositCap / 2));
fundUserAndDepositInVault(userA, fuzzVaultConfig1.vaultId, assetsToDepositA*2);
​
​
//c user stakes index token to get reward share
vm.startPrank(userA);
marketMakingEngine.stake(fuzzVaultConfig1.vaultId, (uint128(IERC20(fuzzVaultConfig1.indexToken).balanceOf(userA)))/2);
vm.stopPrank();
​
//c send WETH market fees from PerpsEngine -> MarketEngine
marketFees = uint128(bound(marketFees, calculateMinOfSharesToStake(fuzzVaultConfig1.vaultId), fuzzVaultConfig1.depositCap / 2));
deal(fuzzVaultConfig1.asset, address(fuzzMarketConfig1.engine), marketFees);
vm.prank(address(fuzzMarketConfig1.engine));
marketMakingEngine.receiveMarketFee(fuzzMarketConfig1.marketId, fuzzVaultConfig1.asset, marketFees);
assertEq(IERC20(fuzzVaultConfig1.asset).balanceOf(address(marketMakingEngine)), marketFees);
​
//c if the asset is not weth, convert the fees to weth
if(fuzzVaultConfig1.asset != marketMakingEngine.workaround_getWethAddress()) {IDexAdapter adapter = getFuzzDexAdapter(adapterIndex);
vm.startPrank(address(fuzzMarketConfig1.engine));
marketMakingEngine.convertAccumulatedFeesToWeth(fuzzMarketConfig1.marketId, fuzzVaultConfig1.asset, adapter.STRATEGY_ID(), bytes(""));
vm.stopPrank();
}
​
//c get user allocation of fees after fees have been deposited
SD59x18 prenarutorewardamount = marketMakingEngine.exposed_getActorValueChange(fuzzVaultConfig1.vaultId, bytes32(uint256(uint160(userA))));
console.log(prenarutorewardamount.unwrap());
​
//c user restakes the index token to get more rewards
vm.startPrank(userA);
marketMakingEngine.stake(fuzzVaultConfig1.vaultId, uint128(IERC20(fuzzVaultConfig1.indexToken).balanceOf(userA)));
vm.stopPrank();
​
//c get user allocation of fees after restaking
SD59x18 postnarutorewardamount = marketMakingEngine.exposed_getActorValueChange(fuzzVaultConfig1.vaultId, bytes32(uint256(uint160(userA))));
console.log(postnarutorewardamount.unwrap());
​
//c users previous rewards are erased even though they havent claimed them using FeesDistributionBranch::claimFees
vm.startPrank(userA);
vm.expectRevert(Errors.NoFeesToClaim.selector);
marketMakingEngine.claimFees(fuzzVaultConfig1.vaultId);
​
}

Impact

Loss of Funds: Users can lose accrued rewards that should rightly belong to them

Tools Used

Manual Review, Foundry

Recommendations

Whenever Distribution::lastValuePerShare is updated, calculate how many rewards the user has already accrued based on actor's Distribution::oldLastValuePerShare and the current global Distribution::valuePerShare.

rewardSoFar =(currentValuePerShare−actor.lastValuePerShare)×actor.shares

Add this value to Distribution::pendingReward. Rather than “burning” these rewards by simply updating lastValuePerShare, save them in the actor’s pendingReward. This effectively acknowledges that the tokens belong to the actor, but the actor hasn’t officially claimed them yet. Once the old, unclaimed rewards are set aside in pendingReward, update the actor’s lastValuePerShare to the latest global Distribution::valuePerShare. This allows new rewards to start accumulating after the user’s stake change without losing prior entitlements.

Updates

Lead Judging Commences

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

Inside VaultRouterBranch if you stake wait some time then stake again makes you lose the rewards.

Support

FAQs

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