Summary
The current implementation of FeeCollector::claimRewards() can cause users to lose a significant portion of their rewards due to an incorrect tracking mechanism.
Vulnerability Details
The function updates userRewards[user] with totalDistributed to track the rewards a user has claimed:
@> uint256 public totalDistributed;
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;
}
In the _calculatePendingRewards() function, user rewards are determined based on their voting power. However, during subsequent reward claims, share < userRewards[user], causing the transaction to revert and preventing users from claiming additional 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;
}
Poc
Add the following test to test/unit/core/collectors/FeeCollector.test.js and execute it:
describe("claimRewards", function () {
beforeEach(async function () {
await network.provider.send("evm_mine");
await feeCollector.connect(user1).collectFee(5000000000000000n, 0);
await feeCollector.connect(user1).collectFee(3000000000000000n, 1);
await feeCollector.connect(user1).collectFee(2000000000000000n, 6);
await feeCollector.connect(owner).distributeCollectedFees();
const user1InitialBalance = await raacToken.balanceOf(user1.address);
const user2InitialBalance = await raacToken.balanceOf(user2.address);
await feeCollector.connect(user1).claimRewards(user1.address);
await feeCollector.connect(user2).claimRewards(user2.address);
const user1BalanceAfterfirstClaimRewards = await raacToken.balanceOf(user1.address);
const user2BalanceAfterfirstClaimRewards = await raacToken.balanceOf(user2.address);
let user1ActualReward = user1BalanceAfterfirstClaimRewards - user1InitialBalance;
let user2ActualReward = user2BalanceAfterfirstClaimRewards - user2InitialBalance;
console.log("user1ActualReward:",user1ActualReward);
console.log("user2ActualReward:",user2ActualReward);
const user1Reward = await feeCollector.userRewards(user1.address);
const user2Reward = await feeCollector.userRewards(user2.address);
console.log("user1Reward:",user1Reward);
console.log("user2Reward:",user2Reward);
});
it("claimRewards revert userReward loss", async function () {
await network.provider.send("evm_mine");
await feeCollector.connect(user1).collectFee(5000000000000000n, 0);
await feeCollector.connect(user1).collectFee(3000000000000000n, 1);
await feeCollector.connect(user1).collectFee(2000000000000000n, 6);
await time.increase(WEEK);
await network.provider.send("evm_mine");
await feeCollector.connect(owner).distributeCollectedFees();
await expect (feeCollector.connect(user1).claimRewards(user1.address)).to.be.revertedWithCustomError(feeCollector, "InsufficientBalance");
await expect (feeCollector.connect(user2).claimRewards(user2.address)).to.be.revertedWithCustomError(feeCollector, "InsufficientBalance");
});
});
output:
FeeCollector
claimRewards
user1ActualReward: 6538810460003384n
user2ActualReward: 3333332170640960n
user1Reward: 10000000000000000n
user2Reward: 10000000000000000n
✔ claimRewards revert userReward loss (93ms)
Note: The actual reward received by the user is significantly smaller than the stored reward value. The second claim attempt fails due to share < userRewards[user].
Impact
The incorrect implementation of the claim function results in users losing a substantial portion of their rewards.
Tools Used
Manual Review
Recommendations
Redesign the implementation of claim rewards
