Submitted on Mar 12th 2024 at 12:49:20 UTC by @oxumarkhatab for Boost | ZeroLend
Report ID: #29262
Report type: Smart Contract
Report severity: Insight
Target: https://github.com/zerolend/governance
Impacts:
- Theft of unclaimed yield
The notifyReward Allows the reward amount for a specific token for multiple times in 14 days Duration. This opens up a weakness of the system using which an attacker can earn more rewards for exact same time as others.
Essentially the attacker can get more of the yield than other users. So Attacker is Thefting the yield ( Just in my opinion ) This is the most relevant impact I found so consider it.
This Vulnerability details section contains the PoC itself too because the only way to show this weakness is to crunch the numbers before you.
In short , if a user has tokens for earning rewards at the time the notifyReward is called , they will get less tokens than a person who stakes tokens at the time after the second notifyReward function call is made to the GuageIncentiveController contract within 14 days and not after 14 days of initial call because second call in the interval [0,14 days) allows setting greater rewardRate for a token ( same amount passed in function call )
that will allow the attacker to gain more rewards after the second notifyReward function call than other people who has staked before the second notifyReward function call
The main lines of concern are setting the rewardRate lines
rewardRate[token] = amount / DURATION;
and
rewardRate[token] = (amount + _left) / DURATION;
Let's see this in action
The rewards calculation for GuageIncentiveController is unfair
function notifyRewardAmount(
IERC20 token,
uint256 amount
) external nonReentrant updateReward(token, address(0)) returns (bool) {
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
} else {
uint256 _remaining = periodFinish[token] - block.timestamp;
uint256 _left = _remaining * rewardRate[token];
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = (amount + _left) / DURATION;
}
lastUpdateTime[token] = block.timestamp;
periodFinish[token] = block.timestamp + DURATION;
// if it is a new incentive, add it to the stack
if (isIncentive[token] == false) {
isIncentive[token] = true;
incentives.push(token);
}
return true;
}LEt's say just after the notifyReward function call , Alice has get's her 100e18 of tokens from somewhere and willing to stake for one day.
LEt's say the notifyReward amount is 10e18
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
}
rewardRate[token] = amount / DURATION;
And the periodFinish is set to block.timestamp+Deadline
She calculates the reward and gets some tokens on A amount of tokens on the rate of 10e18/14 days = 8.2671958e+12
=> rewardRate = 8e12
Now some time passes , as the Duration is 14 days long , a malicious actor Bob can monitor the mempool for notifyReward for the next notifyReward call within next 13.9 days.
When the notifyReward is called ( Let's say for the amount is same as 10e18 ) , block.timestamp < PeriodFinish clearly , so first condition is falsified
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
}
The second condition is executed
else {
uint256 _remaining = periodFinish[token] - block.timestamp;
uint256 _left = _remaining * rewardRate[token];
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = (amount + _left) / DURATION;
}
Let's say notifyReward is called after one day , _remaining = 13 days = 86400*13 = 1,123,200 _left = 1,123,200 * 8e12 = 9e18
clearly , amount > _left , 10e18 > 9e18 so following condition does not execute and new rate is set
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
Carefully see how the rate is being set:
rewardRate[token] = (amount + _left) / DURATION;
rewardRate[token] = (10e18+9e18) / DURATION = 19e18/14 days = 2.1990741e+14 = 2e14
Now when Bob stakes tokens , he gets far more tokens than alice
See 8e12 < 2e14 , This amount is substantially large when accumulate with each passing moment.
so using a simpler formula of elapsedTime and rewardPerToken ( the protocol does slightly different but essense is same)
Alice's rewards = (86400*8e12) = 6.912e+17 = Solidity integer rounding down = 6e17
Bob's rewards = (86400*2e14) = 1.728e+19 = Solidity integer rounding down = 1e19
Thus Bob is able to get more rewards for the same time as Alice's. This is a source of unfair distribution.
-
Unfair rewards distribution between users
-
Theft of rewards
Disclaimer: This is essentially the same PoC in Vulnerability Details section. If you've read that, you can safely ignore this one
The rewards calculation for GuageIncentiveController is unfair
function notifyRewardAmount(
IERC20 token,
uint256 amount
) external nonReentrant updateReward(token, address(0)) returns (bool) {
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
} else {
uint256 _remaining = periodFinish[token] - block.timestamp;
uint256 _left = _remaining * rewardRate[token];
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = (amount + _left) / DURATION;
}
lastUpdateTime[token] = block.timestamp;
periodFinish[token] = block.timestamp + DURATION;
// if it is a new incentive, add it to the stack
if (isIncentive[token] == false) {
isIncentive[token] = true;
incentives.push(token);
}
return true;
}LEt's say just after the notifyReward function call , Alice has get's her 100e18 of tokens from somewhere and willing to stake for one day.
LEt's say the notifyReward amount is 10e18
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
}
rewardRate[token] = amount / DURATION;
And the periodFinish is set to block.timestamp+Deadline
She calculates the reward and gets some tokens on A amount of tokens on the rate of 10e18/14 days = 8.2671958e+12
=> rewardRate = 8e12
Now some time passes , as the Duration is 14 days long , a malicious actor Bob can monitor the mempool for notifyReward for the next notifyReward call within next 13.9 days.
When the notifyReward is called ( Let's say for the amount is same as 10e18 ) , block.timestamp < PeriodFinish clearly , so first condition is falsified
if (block.timestamp >= periodFinish[token]) {
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = amount / DURATION;
}
The second condition is executed
else {
uint256 _remaining = periodFinish[token] - block.timestamp;
uint256 _left = _remaining * rewardRate[token];
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
token.safeTransferFrom(msg.sender, address(this), amount);
rewardRate[token] = (amount + _left) / DURATION;
}
Let's say notifyReward is called after one day , _remaining = 13 days = 86400*13 = 1,123,200 _left = 1,123,200 * 8e12 = 9e18
clearly , amount > _left , 10e18 > 9e18 so following condition does not execute and new rate is set
if (amount < _left) {
return false; // don't revert to help distribute run through its tokens
}
Carefully see how the rate is being set:
rewardRate[token] = (amount + _left) / DURATION;
rewardRate[token] = (10e18+9e18) / DURATION = 19e18/14 days = 2.1990741e+14 = 2e14
Now when Bob stakes tokens , he gets far more tokens than alice
See 8e12 < 2e14 , This amount is substantially large when accumulate with each passing moment.
so using a simpler formula of elapsedTime and rewardPerToken ( the protocol does slightly different but essense is same)
Alice's rewards = (86400*8e12) = 6.912e+17 = Solidity integer rounding down = 6e17
Bob's rewards = (86400*2e14) = 1.728e+19 = Solidity integer rounding down = 1e19
Thus Bob is able to get more rewards for the same time as Alice's. This is a source of unfair distribution.