Skip to content

Latest commit

 

History

History
114 lines (94 loc) · 4.62 KB

001.md

File metadata and controls

114 lines (94 loc) · 4.62 KB

Hidden Crepe Cormorant

Medium

Loss of Rewards After Withdrawing All of One's Funds

Summary

After the depositor withdraws all of their funds and if the remaining rewards are less than the fees, those rewards cannot be claimed and are locked in this contract.

Root Cause

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

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

N/A

Impact

The depositor or protocol loses the remaining rewards, which are locked in this contract. It may less for one depositor, but it is not less for all depositors.

PoC

GovernanceStaker.sol
        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.
720:        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();

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

After the depositer withdraws all of their funds and if the remaing rewards are less than the fees, the _claimReward() function will always revert. Thus, the remaining rewards cannot be claimed and are locked in this contract. FYI, because this depositer's earningpower is zero , the bumpEarningPower() also revert for this depositer.

Mitigation

        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.
+           uint256 _fee = claimFeeParameters.feeAmount;
+           if (_reward <= _fee) {
+               _fee = _reward / 2;
+           }
+720:       uint256 _payout = _reward - _fee;
-720:       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();

            SafeERC20.safeTransfer(REWARD_TOKEN, _claimer, _payout);
-           if (claimFeeParameters.feeAmount > 0) {
+           if (_fee > 0) {
                SafeERC20.safeTransfer(
-                   REWARD_TOKEN, claimFeeParameters.feeCollector, claimFeeParameters.feeAmount
+                   REWARD_TOKEN, claimFeeParameters.feeCollector, _fee
                );
            }
            return _payout;
        }