Skip to content

Latest commit

 

History

History
208 lines (157 loc) · 10.3 KB

041.md

File metadata and controls

208 lines (157 loc) · 10.3 KB

Calm Burlap Mule

Medium

Malicious Staker Can Exploit Outdated Earning Power to Unfairly Accrue or Steal Rewards up to maxBumpTip

Summary

A malicious staker can exploit the staking system to maintain and accrue rewards with an outdated earning power, even if their delegatee becomes ineligible for earning power. This allows the malicious staker to continue accruing rewards unfairly upto maxBumpTip.

Vulnerability Detail

  • When the oracle that updates scores is paused or stale, all delegatees are treated as eligible for earning power
  • Stakers whose delegatees have not reached the eligibility threshold can now accrue rewards during this period
  • When the oracle is restored, their earning power should revert to zero if the delegatee is ineligible for earning power
  • However, a malicious staker can manage to keep the earning power and accrue rewards with the earning power upto maxBumpTip

Attack Path

  • When the oracle that updates scores is paused or stale, a malicious staker stakes and immediately gets an earning power based off the staked amount
  • Just before the oracle state is restored, the malicious staker quickly claims all rewards for the elapsed period with the aim to make their deposit.scaledUnclaimedRewardCheckpoint (i.e. unclaimed rewards) == 0
  • Oracle is restored and the malicious staker's earning power is expected to be bumped back to zero (if their delegatee is ineligible for earning power) by either searcher bots (anyone) or the next time the malicious staker interacts with the staking contract through stakeMore, alterDelegatee, alterClaimer, withdraw, and claimReward
  • The malicious staker avoids interacting with the system to avoid the reset, although since searcher bots or anyone are incentivized to bump stakers earning power through bumpEarningPower function, they can try to update the malicious staker's earning power to zero
  • However, since the malicious staker's deposit.scaledUnclaimedRewardCheckpoint == 0 and the update is to decrease the earning power, any bumpEarningPower function call on the malicious staker's deposit will revert due to this check -> if (_newEarningPower < deposit.earningPower && (_unclaimedRewards - _requestedTip) < maxBumpTip) even if the _unclaimedRewards is enough to cover for the _requestedTip or even if the _requestedTip is zero
  • The check ensures that searcher bots or anyone who wants to decrease the malicious staker's deposit earning power (i.e. back to zero in this case) will have to wait till the malicious staker's unclaimed rewards - the _requestedTip is atleast => maxBumpTip
  • Therefore, this allows the malicious staker to accrue rewards with the outdated earning power to atleast maxBumpTip before searcher bots or anyone can successfully update i.e. decrease the malicious staker's earning power to zero

Note: The amount accrued and gained due to this attack solely depends on the set maxBumpTip

Root Cause

if (_newEarningPower < deposit.earningPower && (_unclaimedRewards - _requestedTip) < maxBumpTip)
    {
      revert GovernanceStaker__InsufficientUnclaimedRewards();
    }
  • The above check ensures that even if the _unclaimedRewards is enough to cover for the _requestedTip or the _requestedTip is zero, as long as the deposit's unclaimed rewards is not => maxBumpTip then the update will always revert. Therefore, this gives the staker's deposit an edge case to always accrue unfair rewards atleast upto the set maxBumpTip

Impact

  • Theft of unclaimed rewards - Malicious stakers can unfairly continue accruing rewards upto maxBumpTip despite their delegatees being ineligible for earning power

Code Snippet

  function bumpEarningPower(
    DepositIdentifier _depositId,
    address _tipReceiver,
    uint256 _requestedTip
  ) external virtual {
    if (_requestedTip > maxBumpTip) revert GovernanceStaker__InvalidTip();


    Deposit storage deposit = deposits[_depositId];


    _checkpointGlobalReward();
    _checkpointReward(deposit);


    uint256 _unclaimedRewards = deposit.scaledUnclaimedRewardCheckpoint / SCALE_FACTOR;


    (uint256 _newEarningPower, bool _isQualifiedForBump) = earningPowerCalculator.getNewEarningPower(
      deposit.balance, deposit.owner, deposit.delegatee, deposit.earningPower
    );
    if (!_isQualifiedForBump || _newEarningPower == deposit.earningPower) {
      revert GovernanceStaker__Unqualified(_newEarningPower);
    }


    if (_newEarningPower > deposit.earningPower && _unclaimedRewards < _requestedTip) {
      revert GovernanceStaker__InsufficientUnclaimedRewards();
    }


    // Note: underflow causes a revert if the requested  tip is more than unclaimed rewards
-    if (_newEarningPower < deposit.earningPower && (_unclaimedRewards - _requestedTip) < maxBumpTip)
-    {
-      revert GovernanceStaker__InsufficientUnclaimedRewards();
-    }


    // Update global earning power & deposit earning power based on this bump
    totalEarningPower =
      _calculateTotalEarningPower(deposit.earningPower, _newEarningPower, totalEarningPower);
    depositorTotalEarningPower[deposit.owner] = _calculateTotalEarningPower(
      deposit.earningPower, _newEarningPower, depositorTotalEarningPower[deposit.owner]
    );
    deposit.earningPower = _newEarningPower.toUint96();


    // Send tip to the receiver
    SafeERC20.safeTransfer(REWARD_TOKEN, _tipReceiver, _requestedTip);
    deposit.scaledUnclaimedRewardCheckpoint =
      deposit.scaledUnclaimedRewardCheckpoint - (_requestedTip * SCALE_FACTOR);
  }
  function _stake(address _depositor, uint256 _amount, address _delegatee, address _claimer)
    internal
    virtual
    returns (DepositIdentifier _depositId)
  {
    _revertIfAddressZero(_delegatee);
    _revertIfAddressZero(_claimer);


    _checkpointGlobalReward();


    DelegationSurrogate _surrogate = _fetchOrDeploySurrogate(_delegatee);
    _depositId = _useDepositId();


-    uint256 _earningPower = earningPowerCalculator.getEarningPower(_amount, _depositor, _delegatee);


    totalStaked += _amount;
    totalEarningPower += _earningPower;
    depositorTotalStaked[_depositor] += _amount;
    depositorTotalEarningPower[_depositor] += _earningPower;
    deposits[_depositId] = Deposit({
      balance: _amount.toUint96(),
      owner: _depositor,
      delegatee: _delegatee,
      claimer: _claimer,
-      earningPower: _earningPower.toUint96(),
      rewardPerTokenCheckpoint: rewardPerTokenAccumulatedCheckpoint,
      scaledUnclaimedRewardCheckpoint: 0
    });
    _stakeTokenSafeTransferFrom(_depositor, address(_surrogate), _amount);
    emit StakeDeposited(_depositor, _depositId, _amount, _amount);
    emit ClaimerAltered(_depositId, address(0), _claimer);
    emit DelegateeAltered(_depositId, address(0), _delegatee);
  }
  function getEarningPower(uint256 _amountStaked, address, /* _staker */ address _delegatee)
    external
    view
    returns (uint256)
  {
-    if (_isOracleStale() || isOraclePaused) return _amountStaked;
    return _isDelegateeEligible(_delegatee) ? _amountStaked : 0;
  }

Tool used

Manual Review

Recommendation

These checks should be modified and simplified. The important check is to ensure the deposit's unclaimed rewards is enough to cover for the requested tip.

  function bumpEarningPower(
    DepositIdentifier _depositId,
    address _tipReceiver,
    uint256 _requestedTip
  ) external virtual {
    if (_requestedTip > maxBumpTip) revert GovernanceStaker__InvalidTip();


    Deposit storage deposit = deposits[_depositId];


    _checkpointGlobalReward();
    _checkpointReward(deposit);


    uint256 _unclaimedRewards = deposit.scaledUnclaimedRewardCheckpoint / SCALE_FACTOR;


    (uint256 _newEarningPower, bool _isQualifiedForBump) = earningPowerCalculator.getNewEarningPower(
      deposit.balance, deposit.owner, deposit.delegatee, deposit.earningPower
    );
    if (!_isQualifiedForBump || _newEarningPower == deposit.earningPower) {
      revert GovernanceStaker__Unqualified(_newEarningPower);
    }

-    if (_newEarningPower > deposit.earningPower && _unclaimedRewards < _requestedTip) {
-      revert GovernanceStaker__InsufficientUnclaimedRewards();
-    }

    // Note: underflow causes a revert if the requested  tip is more than unclaimed rewards
-    if (_newEarningPower < deposit.earningPower && (_unclaimedRewards - _requestedTip) < maxBumpTip)
-    {
-      revert GovernanceStaker__InsufficientUnclaimedRewards();
-    }

+    if (_unclaimedRewards < _requestedTip) revert GovernanceStaker__InsufficientUnclaimedRewards();

    // Update global earning power & deposit earning power based on this bump
    totalEarningPower =
      _calculateTotalEarningPower(deposit.earningPower, _newEarningPower, totalEarningPower);
    depositorTotalEarningPower[deposit.owner] = _calculateTotalEarningPower(
      deposit.earningPower, _newEarningPower, depositorTotalEarningPower[deposit.owner]
    );
    deposit.earningPower = _newEarningPower.toUint96();


    // Send tip to the receiver
    SafeERC20.safeTransfer(REWARD_TOKEN, _tipReceiver, _requestedTip);
    deposit.scaledUnclaimedRewardCheckpoint =
      deposit.scaledUnclaimedRewardCheckpoint - (_requestedTip * SCALE_FACTOR);
  }