Big Fuzzy Salmon
Medium
A check in GovernanceStaker.claimReward()
will always cause DOS if alterClaimer()
is ever used.
if (deposit.claimer != msg.sender && deposit.owner != msg.sender) {
revert GovernanceStaker__Unauthorized("not claimer or owner", msg.sender);
}
The purpose of alterClaimer()
is to grant a user the ability to change his deposit.claimer
/// @notice For an existing deposit, change the claimer account which has the right to
/// withdraw staking rewards.
/// @param _depositId Unique identifier of the deposit which will have its claimer altered.
/// @param _newClaimer Address of the new claimer.
/// @dev The new claimer may not be the zero address. The message sender must be the owner of
/// the deposit.
function alterClaimer(DepositIdentifier _depositId, address _newClaimer) external virtual {//@audit-issue using alterClaimer() will Dos claimReward()
Deposit storage deposit = deposits[_depositId];
_revertIfNotDepositOwner(deposit, msg.sender);
_alterClaimer(deposit, _depositId, _newClaimer);
}
When a user uses this function, he will not be able to successfully call GovernanceStaker.claimReward()
/// @notice Claim reward tokens earned by a given deposit. Message sender must be the claimer
/// address of the deposit. Tokens are sent to the claimer address.
/// @param _depositId Identifier of the deposit from which accrued rewards will be claimed.
/// @return Amount of reward tokens claimed, after the fee has been assessed.
function claimReward(DepositIdentifier _depositId) external virtual returns (uint256) {
Deposit storage deposit = deposits[_depositId];
if (deposit.claimer != msg.sender && deposit.owner != msg.sender) {//@audit
revert GovernanceStaker__Unauthorized("not claimer or owner", msg.sender);
}
return _claimReward(_depositId, deposit, msg.sender);
}
check the if statement which the @audit tag is on.. It will cause reverts when claimer is changed from original claimer which will be owner to a new address.
reverts in GovernanceStaker.claimReward()
Breaks core functionality of alterClaimer() because it can't be used. and if it is used GovernanceStaker.claimReward()
will revert
https://github.com/sherlock-audit/2024-11-tally/blob/main/staker/src/GovernanceStaker.sol#L409
Manual Review
The alterClaimer()
function should be removed or the check in GovernanceStaker.claimReward()
should be changed to
if ( deposit.owner != msg.sender) {//@audit
revert GovernanceStaker__Unauthorized("not claimer or owner", msg.sender);
}