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

Failure to update `lastClaimTime` mapping when users claim rewards in FeeCollector Causes Time-Based Reward Calculation Issues

tinnohofficial

Summary

The FeeCollector contract maintains a lastClaimTime mapping that tracks when users last claimed rewards, but this mapping is never updated when claims occur. The mapping has an associated internal function _updateLastClaimTime() that's never called, making the time tracking functionality completely ineffective.

Vulnerability Details

The issue centers around the contract's reward distribution and claiming mechanism. Let's break down the relevant components:

The contract has a time-tracking mapping:

mapping(address => uint256) private lastClaimTime;

And an internal function to update it, _updateLastClaimTime:

// @audit function not invoked when claiming rewards.
function _updateLastClaimTime(address user) internal {
lastClaimTime[user] = block.timestamp;
}

However, in the claimRewards function where users claim their rewards, there's no call to _updateLastClaimTime:

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;
}

This creates several issues;

The FeeCollector contract also uses the TimeWeightedAverage library for distribution periods, but without proper time tracking, the weight calculations may be inaccurate.

The FeeCollector also loses the ability to track when users are claiming rewards, which could be important for prevention of reward exploitation, implementation of time-based restrictions or bonuses or tracking of reward distributions

The current reward calculation relies solely on the difference between a user's share of totalDistributed and their userRewards balance:

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

  1. Alice has voting power and is eligible for rewards

  2. A distribution period starts with 1000 tokens

  3. Alice claims her rewards

  4. lastClaimTime[Alice] remains at 0 despite the claim

  5. Another distribution occurs

  6. Contract has no way to determine when Alice last claimed, potentially affecting time-weighted calculations

Impact

  • Loss of time-based tracking functionality

  • Inaccurate reward calculations if time-weighted mechanics are added

  • Wasted gas on unused storage

  • Potential for future integration issues

Tools Used

Manual code review

Recommendations

properly implement time tracking in the claimRewards function:

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;
_updateLastClaimTime(user); // @audit Add this line
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}
Updates

Lead Judging Commences

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

_updateLastClaimTime not properly used to track rewards claim time

Support

FAQs

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