Voter::finalize() incorrect rewards distribution due to transfering WETH before calling distributor::setTokensPerInterval()
Summary
Voter::finalize() transfers the WETH, and then calls distributor.setTokensPerInterval(). However, this order of operations is incorrect and it will lead to the previous reward rate being applied for the transferred WETH, doing an instantaneous rewards increase that will be arbitraged and stolen.
As can be seen below, RewardsDistributor::setTokensPerInterval() calls RewardTracker::updateRewards(), which calls RewardsDistributor::distribute(). Now, if pendingRewards() will apply the last tokensPerInterval, accrue for the last time called, and send to users if there is enough balance. Due to having sent the WETH before calling setTokensPerInterval(), it will instantly distribute this rewards as soon as the function is called, and there won't be enough rewards for the next week. Thus, past stakers (and bots or attackers that knew about this) will steal instant rewards, and honest stakers will not collect them over the full week.
function setTokensPerInterval(uint256 _amount) external onlyAdmin {
require(lastDistributionTime != 0, "RewardDistributor: invalid lastDistributionTime");
IRewardTracker(rewardTracker).updateRewards();
tokensPerInterval = _amount;
emit TokensPerIntervalChange(_amount);
}
function pendingRewards() public view override returns (uint256) {
if (block.timestamp == lastDistributionTime) {
return 0;
}
uint256 timeDiff = block.timestamp.sub(lastDistributionTime);
return tokensPerInterval.mul(timeDiff);
}
function distribute() external override returns (uint256) {
require(msg.sender == rewardTracker, "RewardDistributor: invalid msg.sender");
uint256 amount = pendingRewards();
if (amount == 0) { return 0; }
lastDistributionTime = block.timestamp;
uint256 balance = IERC20(rewardToken).balanceOf(address(this));
if (amount > balance) { amount = balance; }
IERC20(rewardToken).safeTransfer(msg.sender, amount);
emit Distribute(amount);
return amount;
} function updateRewards() external override nonReentrant {
_updateRewards(address(0));
}
function _updateRewards(address _account) private {
uint256 blockReward = IRewardDistributor(distributor).distribute();
...Root Cause
In Voter.sol:262, it transfers WETH before calling setTokensPerInterval().
Internal Pre-conditions
None
External Pre-conditions
None
Attack Path
- Admin finalizes an epoch, sends WETH and sets the tokens per interval to distribute this amount of WETH over 1 week.
- Due to the logic above, part of this WETH, if not all, will be consumed instantly and rewards won't be distributed over the full week.
Impact
Stolen rewards.
Mitigation
First call setTokensPerInterval(), then transfer the WETH.