Long Rouge Jay - When the deposit owner claims it, the reward is sent to the deposit owner instead of the designated deposit claimer.

[sherlock-admin4]

Long Rouge Jay

Medium

When the deposit owner claims it, the reward is sent to the deposit owner instead of the designated deposit claimer.

Summary

When the deposit owner calls claimReward, the reward is sent to the deposit owner instead of the designated deposit claimer, which is not as expected and causes the deposit claimer to lose of rewards.

Vulnerability Detail

According to the deposit function, when a user stakes tokens into a new deposit, he can designates a _claimer address that will accrue rewards for the stake. As indicated in GovernanceStaker.sol:L344, the rewards for the deposit should be sent to the claimer.

    /// @notice Method to stake tokens to a new deposit. The caller must pre-approve the staking
    /// contract to spend at least the would-be staked amount of the token.
    /// @param _amount Quantity of the staking token to stake.
    /// @param _delegatee Address to assign the governance voting weight of the staked tokens.
    /// @param _claimer Address that will accrue rewards for this stake.
344:/// @return _depositId Unique identifier for this deposit.
    /// @dev Neither the delegatee nor the claimer may be the zero address. The deposit will be
    /// owned by the message sender.
    function stake(uint256 _amount, address _delegatee, address _claimer)
      external
      virtual
      returns (DepositIdentifier _depositId)
    {
      _depositId = _stake(msg.sender, _amount, _delegatee, _claimer);
    }

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

According to the claimReward function, to claim the rewards, the caller needs to be either the deposit claimer or the deposit owner. Following the authorization check, the _claimReward function will be invoked to calculate the accumulated rewards and send them to the msg.sender (GovernanceStaker.sol:L412). If it is called by the deposit owner, then msg.sender will be the deposit owner.

    function claimReward(DepositIdentifier _depositId) external virtual returns   (uint256) {
      Deposit storage deposit = deposits[_depositId];
409:  if (deposit.claimer != msg.sender && deposit.owner != msg.sender) {
        revert GovernanceStaker__Unauthorized("not claimer or owner", msg.sender);
      }
412:  return _claimReward(_depositId, deposit, msg.sender);
    }

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

Therefore, if the deposit owner claims the rewards, the rewards will be sent to the deposit owner instead of the deposit claimer, which is inconsistent with the description of the stake function, and causes the deposit claimer to lose of rewards.

Impact

When deposit owner claims the rewards, the rewards will be sent to the deposit owner instead of the deposit claimer, which is inconsistent with the role of the deposit claimer, and causes the deposit claimer to lose of rewards.

Code Snippet

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

Tool used

Manual Review

Recommendation

    function claimReward(DepositIdentifier _depositId) external virtual returns   (uint256) {
      Deposit storage deposit = deposits[_depositId];
      if (deposit.claimer != msg.sender && deposit.owner != msg.sender) {
        revert GovernanceStaker__Unauthorized("not claimer or owner", msg.sender);
      }
-     return _claimReward(_depositId, deposit, msg.sender);
+     return _claimReward(_depositId, deposit, deposit.claimer);
    }