Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Reward Calculation Due to Flawed Return Logic in FeeCollector's `calculatePendingRewards` Function Causes Users To Get Lesser Rewards

uddercover

Summary

When a user calls FeeCollector::claimReward, the FeeCollector::_calculatePendingRewards function incorrectly subtracts previous FeeCollector::totalDistributed rewards from the calculated user share in the return statement of the function. This results in users receiving fewer rewards than they should based on their voting power percentage.

Vulnerability Details

The issue is in the _calculatePendingRewards function:

function _calculatePendingRewards(address user) public 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 example below describes why the return statement is problematic. This is for a user with 50% voting power.

For the first distribution:

  • Total Distributed = 1000 tokens

  • User has 50% voting power

  • User's share = (1000 * 50%) / 100% = 500

  • Returned amount to claim = 500 - 0 = 500 // 0 is because at this time userRewards[user] is still 0

  • User claims 500 tokens

  • userRewards[user] is set to 1000

For the second distribution:

  • New tokens distributed = 2000

  • Total Distributed = 3000 (1000 + 2000)

  • User still has 50% voting power

  • User's share calculation:

  • share = (3000 * 50%) / 100% = 1500

  • Returned amount to claim = 1500 - 1000 = 500

  • But they should get: (2000 * 50%) / 100% = 1000 tokens from the new distribution

POC

To use foundry in the codebase, follow the hardhat guide here: Foundry-Hardhat hybrid integration by Nomic foundation

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {FeeCollector} from "../../../../contracts/core/collectors/FeeCollector.sol";
import {RAACToken, PercentageMath} from "../../../../contracts/core/tokens/RAACToken.sol";
import {veRAACToken} from "../../../../contracts/core/tokens/veRAACToken.sol";
import {Test, console} from "forge-std/Test.sol";
​
contract TestSuite is Test {
FeeCollector feeCollector;
RAACToken raacToken;
veRAACToken veRAACTok;
address treasury;
address repairFund;
address admin;
uint256 initialSwapTaxRate = 100; //1%
uint256 initialBurnTaxRate = 50; //0.5%
​
function setUp() public {
treasury = makeAddr("treasury");
repairFund = makeAddr("repairFund");
admin = makeAddr("admin");
​
raacToken = new RAACToken(admin, initialSwapTaxRate, initialBurnTaxRate);
veRAACTok = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(address(raacToken), address(veRAACTok), treasury, repairFund, admin);
​
vm.startPrank(admin);
raacToken.setFeeCollector(address(feeCollector));
raacToken.setMinter(admin);
vm.stopPrank();
}
​
function testUsersLoseSomeFeesDueToBadReturnStatement() public {
address user = makeAddr("user");
address user2 = makeAddr("user2");
uint256 firstAmountDistributed = 1e18;
uint256 secondAmountDistributed = 2e18;
uint256 maxDuration = veRAACTok.MAX_LOCK_DURATION();
​
//admin mints tokens and calls collect fee
vm.startPrank(admin);
raacToken.mint(admin, firstAmountDistributed + secondAmountDistributed);
raacToken.mint(user, firstAmountDistributed);
raacToken.mint(user2, firstAmountDistributed);
raacToken.approve(address(feeCollector), firstAmountDistributed + secondAmountDistributed);
feeCollector.collectFee(firstAmountDistributed, 0);
vm.stopPrank();
​
//users mint veRaacTokens with greatest duration, to each have 50% of total voting power
vm.startPrank(user);
raacToken.approve(address(veRAACTok), firstAmountDistributed);
veRAACTok.lock(firstAmountDistributed, maxDuration);
vm.stopPrank();
​
vm.startPrank(user2);
raacToken.approve(address(veRAACTok), firstAmountDistributed);
veRAACTok.lock(firstAmountDistributed, maxDuration);
vm.stopPrank();
​
//admin distributes fees
vm.prank(admin);
feeCollector.distributeCollectedFees(); //a bug in this function prevents this from working i.e `if (contractBalance < totalFees) revert InsufficientBalance();` in _processDistributions function as contractBalance would always be less than tracked fees because of the raacToken feeOnTransfer.
​
//user claims, first time so userRewards[user] is set to greater than 0
vm.prank(user);
feeCollector.claimRewards(user);
console.log("User voting power at first claim: ", veRAACTok.getVotingPower(user));
console.log("Total voting power at first claim: ", veRAACTok.getTotalVotingPower());
​
//admin calls collectFee second time
vm.warp(7 days + 1);
vm.prank(admin);
feeCollector.collectFee(secondAmountDistributed, 0);
​
//get shares for fee distribution before they are deleted in distributeCollectedFees
uint256 totalFees = feeCollector._calculateTotalFees(); //function modified to public for ease of access
uint256[4] memory shares = feeCollector._calculateDistribution(totalFees); //function modified to public for ease of access
​
//admin distributes collected fees, second time
vm.prank(admin);
feeCollector.distributeCollectedFees();
​
//user share is lower than expected for their voting power
​
uint256 userVotingPower = veRAACTok.getVotingPower(user);
uint256 totalVotingPower = veRAACTok.getTotalVotingPower();
console.log("User voting power at second claim: ", userVotingPower);
console.log("Total voting power at second claim: ", totalVotingPower);
​
uint256 expectedUserShareSecond = (shares[0] * userVotingPower) / totalVotingPower; // Shares are expected to be dependent on the percentage voting power of users. shares[0] is the total allocated amount for veRaac holders, for the second distribution.
uint256 actualUserShareSecond = feeCollector._calculatePendingRewards(user);
​
assertGt(expectedUserShareSecond, actualUserShareSecond);
}
}

Impact

Users lose a large part of their expected rewards

Tools Used

Manual review, foundry test suite

Recommendations

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;
// Calculate share of new distributions only
+ uint256 newDistributions = totalDistributed - userRewards[user];
+ uint256 share = (newDistributions * userVotingPower) / totalVotingPower;
+ return share;
- uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
- return share > userRewards[user] ? share - userRewards[user] : 0;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

FeeCollector::claimRewards sets `userRewards[user]` to `totalDistributed` seriously grieving users from rewards

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.