Summary
The FeeCollector::_calculatePendingRewards function uses wrong formula to calculate user share. This can lead to loss of funds for the user.
Vulnerability Details
We have to keep in mind that the userRewards is the amount of total reward that the user has already claimed, not only user's share of the total reward.
/contracts/core/collectors/FeeCollector.sol
function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
@> userRewards[user] = totalDistributed;
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}
Then if voting power are not changed and second distribution have less or equal amount of fee to distribute than previous, the calculation will leave user with zero reward.
Asuming that the first and the second distribution was 1000 each. And we have 2 users with same voting power. After first distribution user have claimed their reward. So after second distributeCollectedFees call and before claimRewards call state will be:
totalVotingPower = 2;
userVotingPower = 1;
totalDistributed = 2000;
userRewards[user] = 1000;
Then user's share will be calculated as:
share = (2000 * 1) / 2 = 1000;
return share > userRewards[user] ? share - userRewards[user] : 0;
return 1000 > 1000 ? 1000 - 1000 : 0;
return 0;
/contracts/core/collectors/FeeCollector.sol
function _calculatePendingRewards(address user) internal view returns (uint256) {
uint256 userVotingPower = veRAACToken.getVotingPower(user);
if (userVotingPower == 0) return 0;
uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
if (totalVotingPower == 0) return 0;
@> uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
@> return share > userRewards[user] ? share - userRewards[user] : 0;
}
PoC (foundry)
pragma solidity 0.8.28;
import {Test} from "forge-std/Test.sol";
import {RAACToken} from "src/core/tokens/RAACToken.sol";
import {veRAACToken} from "src/core/tokens/veRAACToken.sol";
import {FeeCollector} from "src/core/collectors/FeeCollector.sol";
contract FeeCollectorTestShareCalculation is Test {
RAACToken public raacToken;
veRAACToken public veToken;
FeeCollector public feeCollector;
function setUp() public {
raacToken = new RAACToken(address(this), 0, 0);
raacToken.setMinter(address(this));
veToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(
address(raacToken),
address(veToken),
makeAddr("treasury"),
makeAddr("repairFund"),
address(this));
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(veToken), true);
raacToken.manageWhitelist(address(this), true);
}
function test_shareCalculation() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
raacToken.mint(address(this), 2000);
raacToken.mint(alice, 1000);
raacToken.mint(bob, 1000);
vm.startPrank(alice);
raacToken.approve(address(veToken), 1000);
veToken.lock(1000, 365 days);
vm.stopPrank();
vm.startPrank(bob);
raacToken.approve(address(veToken), 1000);
veToken.lock(1000, 365 days);
vm.stopPrank();
raacToken.approve(address(feeCollector), 2000);
feeCollector.collectFee(1000, 0);
feeCollector.distributeCollectedFees();
vm.prank(alice);
feeCollector.claimRewards(alice);
vm.prank(bob);
feeCollector.claimRewards(bob);
skip(8 days);
feeCollector.collectFee(1000, 0);
feeCollector.distributeCollectedFees();
vm.prank(alice);
feeCollector.claimRewards(alice);
}
}
Impact
User can lose rewards.
Recommendations
Share must be calculated from the user's unpaid rewards, not from the total rewards.
function _calculatePendingRewards(address user) internal view returns (uint256) {
uint256 userVotingPower = veRAACToken.getVotingPower(user);
if (userVotingPower == 0) return 0;
uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
if (totalVotingPower == 0) return 0;
- uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
- return share > userRewards[user] ? share - userRewards[user] : 0;
+ return ((totalDistributed - userRewards[user]) * userVotingPower) / totalVotingPower;
}
