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

An attacker can prevent stakers from unstaking by updating `lastValuePerShare` continuously.

qpzm

Summary

VaultRouterBranch.unstake reverts if the claimable amount of msg.sender, i.e. amountToClaimX18 is not 0.

UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId).intoUD60x18();
​
// reverts if the claimable amount is NOT 0
if (!amountToClaimX18.isZero()) revert Errors.UserHasPendingRewards(actorId, amountToClaimX18.intoUint256());

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/branches/VaultRouterBranch.sol#L599-L602

Market.receiveWethReward updates Market.Data.wethRewardPerVaultShare which changes amountToClaimX18.
https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/leaves/Market.sol#L524-L525

Market.receiveWethReward is called only in FeeDistributionBranch._handleWethRewardDistribution.
https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/branches/FeeDistributionBranch.sol#L392

FeeDistributionBranch._handleWethRewardDistribution is called in FeeDistributionBranch.receiveMarketFee.
https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/branches/FeeDistributionBranch.sol#L112

It is only callable by allowed engines, but not currently used. We can guess WETH fee will be incremented by other
traders' opening or closing positions.

Vulnerability Details

Add this test at the end of Unstake_Integration_Test.
It shows that VaultRouterBranch.unstake reverts if unclaimed fee exists.

// @audit Modified `test_stakerLosesUnclaimedRewardsWhenUnstakingBeforeClaiming`
// If unclaimed fee exists, VaultRouterBranch.unstake reverts.
function testPOC_UnstakeFailAfterReceiveMarketFee() external {
// ensure valid vault and load vault config
uint128 vaultId = WETH_CORE_VAULT_ID;
VaultConfig memory fuzzVaultConfig = getFuzzVaultConfig(vaultId);
​
// ensure valid deposit amount and perform the deposit
address user = users.naruto.account;
uint128 assetsToDeposit = uint128(calculateMinOfSharesToStake(vaultId));
fundUserAndDepositInVault(user, vaultId, assetsToDeposit);
​
// save and verify pre state
UnstakeState memory preStakeState =
_getUnstakeState(user, vaultId, IERC20(fuzzVaultConfig.asset), IERC20(fuzzVaultConfig.indexToken));
assertGt(preStakeState.stakerVaultBal, 0, "Staker vault balance > 0 after deposit");
​
// perform the stake
vm.startPrank(user);
marketMakingEngine.stake(vaultId, preStakeState.stakerVaultBal);
​
// sent WETH market fees from PerpsEngine -> MarketEngine
uint256 marketFees = 1e18;
deal(fuzzVaultConfig.asset, address(perpMarketsCreditConfig[ETH_USD_MARKET_ID].engine), marketFees);
changePrank({ msgSender: address(perpMarketsCreditConfig[ETH_USD_MARKET_ID].engine) });
vm.expectEmit({ emitter: address(marketMakingEngine) });
emit FeeDistributionBranch.LogReceiveMarketFee(fuzzVaultConfig.asset, ETH_USD_MARKET_ID, marketFees);
marketMakingEngine.receiveMarketFee(ETH_USD_MARKET_ID, fuzzVaultConfig.asset, marketFees);
assertEq(IERC20(fuzzVaultConfig.asset).balanceOf(address(marketMakingEngine)), marketFees);
​
// verify the staker has earned rewards which are not yet claimed
changePrank({ msgSender: user });
assertEq(
marketMakingEngine.getEarnedFees(vaultId, user), 899_999_999_999_999_999, "Staker has pending rewards"
);
vm.expectRevert(abi.encodeWithSelector(Errors.UserHasPendingRewards.selector, user, 899999999999999999));
marketMakingEngine.unstake(vaultId, preStakeState.stakerVaultBal);
}

Impact

FeeDistributionBranch.receiveMarketFee
will be called by other engines. An attacker can prevent stakers from unstaking by calling the functions updating
lastValuePerShare continuously.

Tools Used

Foundry.

Recommendations

Add a new field to store the unclaimed rewards and update rewards in Distribution._updateLastValuePerShare.

struct Actor {
uint128 shares;
int256 lastValuePerShare;
+ uint256 rewards;
}

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/leaves/Distribution.sol#L17-L20

Reference

  • https://github.com/Synthetixio/synthetix/blob/d7b5c386ffbd0a53a129cfbd8a683a6a3f62cc36/contracts/StakingRewards.sol#L29

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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