Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: high
Valid

Unclaimed WETH Fees are Lost When Staking Additional Shares

Summary

Users can lose unclaimed WETH fees when staking additional shares due to improper handling of fee accumulation state in the staking process. The vulnerability stems from the stake() function updating accumulated fees without first ensuring pending fees are claimed.

Links to affected code

  • VaultRouterBranch.sol:380~420

  • FeeDistributionBranch.sol:284~316

  • Distribution.sol:6164, 93104

Vulnerability details

Finding description and impact

I have identified that users can inadvertently lose their unclaimed WETH fees when staking additional shares. This occurs due to a critical interaction between the fee claiming and staking mechanisms.

In the FeeDistributionBranch.sol contract, the claimFees() function appropriately handles fee distribution by updating the actor's lastValuePerShare through the accumulateActor() call at L300.

The core issue lies in the Distribution.sol contract where _updateLastValuePerShare() at L103 resets the accumulated fees when updating the actor's state:

actor.lastValuePerShare = newActorShares.eq(UD60x18_ZERO) ? int256(0) : self.valuePerShare;

However, the stake() function in VaultRouterBranch.sol calls accumulateActor() without first ensuring that pending fees are claimed. This sequence of operations effectively zeroes out any unclaimed fees the user had accumulated before staking additional shares.

This vulnerability results in a direct financial loss for users who stake additional shares before claiming their pending WETH fees.

Recommended mitigation steps

I recommend modifying the stake() function in VaultRouterBranch.sol to prevent users from staking when they have unclaimed fees:

function stake(uint128 vaultId, uint128 shares) external {
...........................
// cast actor address to bytes32
bytes32 actorId = bytes32(uint256(uint160(msg.sender)));
+ // get the claimable amount of fees
+ UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId).intoUD60x18();
+ // reverts if the claimable amount != 0
+ if (!amountToClaimX18.isZero()) revert Errors.UserHasPendingRewards(actorId, amountToClaimX18.intoUint256());
// accumulate the actor's pending reward before staking
wethRewardDistribution.accumulateActor(actorId);
..........................
}

This change ensures users must claim their pending rewards before staking additional shares, preventing any accidental loss of funds.

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 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.