Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Malicious user could game the protocol to their advantage

Summary

StakingPool allows users to game the reward system by withdrawing funds before strategy rewards are updated, or by strategically sidestepping and timing deposits/withdrawals to capture a disproportionate share of rewards or avoid losses.

Vulnerability Details

The meat of the matter is from the separation between the updateStrategyRewards function and the withdraw function. The withdraw function does not automatically update strategy rewards before processing a withdrawal. This means that a user's withdrawal amount is calculated based on potentially outdated reward information.

As hinted in the docs:

The liquid staking token issued by StakingPool is a rebasing token so rewards will be automatically distributed to stakers without any action on their part. This is possible by using a "shares" accounting model where a share represents a certain percentage ownership of the pool.

Instead the contract allowss:

-- Withdrawals without updating strategy rewards leading to outdated reward distribution.
-- Selective updating of strategy rewards, which could be exploited to update only profitable strategies.
-- No cooldown period for newly deposited funds, enabling quick deposit-update-withdraw cycles.

Key points to note here:

  1. The totalStaked variable, which is impoetant for calculating user balances, is only updated in the _updateStrategyRewards function.

550: // update totalStaked if there was a net change in deposits
551: if (totalRewards != 0) {
552: totalStaked = uint256(int256(totalStaked) + totalRewards);
553: }
  1. The withdraw function does not call updateStrategyRewards before processing the withdrawal.

  2. Users can call updateStrategyRewards selectively for specific strategies potentiallly allowing them to update only profitable strategies.

This setup allows users to:

  1. Withdraw funds before losses are accounted for.

  2. Miss out on rewards if they withdraw before rewards are updated.

  3. Game the system by depositing large amounts, updating only profitable strategies, and then immediately withdrawing.

Consider this scenario:

-- The StakingPool has 1,000 tokens total staked.
-- Alice has staked 100 tokens (10% of the pool).
-- Bob has staked 900 tokens (90% of the pool).
-- The pool has one strategy, Strategy S.

Day 1:
-- Strategy S earns 100 tokens in rewards.
-- The total value in the pool is now 1,100 tokens, but totalStaked still shows 1,000 because updateStrategyRewards hasn't been called.

Day 2:

  1. Alice checks her balance:
    -- balanceOf(Alice) still returns 100 tokens (10% of 1,000) because updateStrategyRewards hasn't been called.

  2. Alice calls withdraw(100):
    -- She receives 100 tokens, her original stake.
    -- Alice misses out on her share of the rewards (10 tokens).

Day 3:

  1. Bob or another user calls updateStrategyRewards:
    -- totalStaked is updated to 1,100 tokens.
    -- The remaining 1,000 tokens in the pool now represent 1,100 tokens of value.

  2. Bob checks his balance:
    -- balanceOf(Bob) now returns 1,100 tokens (100% of 1,100).

  3. Bob calls withdraw(1100):
    -- He receives 1,100 tokens, his original stake plus all the rewards.

Outcome:
-- Alice withdrew 100 tokens (her original stake).
-- Bob withdrew 1,100 tokens (his stake plus all rewards).
-- Bob effectively got Alice's share of the rewards because Alice withdrew before the rewards were accounted for.

As for losses, imagine if the strategy had lost 100 tokens instead. If Alice withdrew before updateStrategyRewards was called, she would get her full 100 tokens back, while Bob would bear all the losses when he eventually withdraws

Then suppose we have Carol observing this behavior. She could:
-- Wait for Strategy S to accumulate significant rewards.
-- Quickly deposit a large amount, say 9,000 tokens.
-- Immediately call updateStrategyRewards.
-- Withdraw her stake plus a disproportionate share of the rewards.
-- Leave before any potential losses occur.

Impact

Unfair distribution of rewards and losses among users & some users could avoid losses at the expense of others.

Tools used

Manual review

Recommendations

Implement a way to automatically update rewards for all strategies before processing any withdrawal. Consider introducing a cooldown period for newly deposited funds to prevent quick deposit-update-withdraw cycles. Also add a time-weighted average or snapshot system for reward distribution to ensure fairer allocation over time. Lastly, update all strategies simultaneously rather than allowing selective updates to prevent cherry-picking of profitable strategies.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[INVALID] `getSharesByStake()` does not account for up-to-date rewards or losses from staking strategies unless the `updateStrategyRewards()` function is called

Support

FAQs

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