004.md

Howling Amethyst Moth

Medium

Possible fee payment bypass by user

Summary

The contract will be deployed on ETH, so a user can openly create or use a flashbot as done by the protocol or directly call if call has not been made yet, to claim their fee by calling bumpEarningPower, successfully bypassing the fee collection mechanism in claim reward.

Vulnerability Detail

A fee is charged on every withdrawal of reward token in the contract but the protocol fails to take a fee from the amount sent out, this will create a RACE condition , allowing the deposit owner/ claim address to successfully claim rewards without paying the fee.

Anyone can call

 /// @notice A function that a bumper can call to update a deposit's earning power when a
  /// qualifying change in the earning power is returned by the earning power calculator. A
  /// deposit's earning power may change as determined by the algorithm of the current earning power
  /// calculator. In order to incentivize bumpers to trigger these updates a portion of deposit's
  /// unclaimed rewards are sent to the bumper.
  /// @param _depositId The identifier for the deposit that needs an updated earning power.
  /// @param _tipReceiver The receiver of the reward for updating a deposit's earning power.
  /// @param _requestedTip The amount of tip requested by the third-party.
  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();

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

Fee boycott

 /// @notice Internal convenience method which claims earned rewards.
  /// @return Amount of reward tokens claimed, after the claim fee has been assessed.
  /// @dev This method must only be called after proper authorization has been completed.
  /// @dev See public claimReward methods for additional documentation.
  function _claimReward(DepositIdentifier _depositId, Deposit storage deposit, address _claimer)
    internal
    virtual
    returns (uint256)
  {
    _checkpointGlobalReward();
    _checkpointReward(deposit);

    uint256 _reward = deposit.scaledUnclaimedRewardCheckpoint / SCALE_FACTOR;
    // Intentionally reverts due to overflow if unclaimed rewards are less than fee.

@audit>>>     uint256 _payout = _reward - claimFeeParameters.feeAmount;
   
    if (_payout == 0) return 0;

    // retain sub-wei dust that would be left due to the precision loss
    deposit.scaledUnclaimedRewardCheckpoint =
      deposit.scaledUnclaimedRewardCheckpoint - (_reward * SCALE_FACTOR);
    emit RewardClaimed(_depositId, _claimer, _payout);

    uint256 _newEarningPower =
      earningPowerCalculator.getEarningPower(deposit.balance, deposit.owner, deposit.delegatee);

    totalEarningPower =
      _calculateTotalEarningPower(deposit.earningPower, _newEarningPower, totalEarningPower);
    depositorTotalEarningPower[deposit.owner] = _calculateTotalEarningPower(
      deposit.earningPower, _newEarningPower, depositorTotalEarningPower[deposit.owner]
    );
    deposit.earningPower = _newEarningPower.toUint96();



@audit>>>     SafeERC20.safeTransfer(REWARD_TOKEN, _claimer, _payout);

@audit>>>     if (claimFeeParameters.feeAmount > 0) {
      SafeERC20.safeTransfer(
        REWARD_TOKEN, claimFeeParameters.feeCollector, claimFeeParameters.feeAmount
      );
    }
    return _payout;
  }

The fee collection in v3factory in Unisataker also implements a minus 1 on fess collected this issue as reported by auditor in the Unistaker audit, but this contract fails to remove any form of fees at all, making the call more juicy for users with rewards less than MAX as they can just withdraw all their reward tokens without paying any fee

Impact

Users with enough reward amount to claim can take advantage to withdraw all their tokens and bypass the fee collection.

Code Snippet

https://github.com/sherlock-audit/2024-11-tally/blob/main/staker/src/GovernanceStaker.sol#L462-L514

https://github.com/sherlock-audit/2024-11-tally/blob/main/staker/src/GovernanceStaker.sol#L706-L745

Tool used

Manual Review

Recommendation

Take a fee from the bumpEarningPower call also, a new fee model can be created for this or use the same fee from Claimreward.