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

`veRAACToken.lock()` Overwrite `state.locks[user]` Technically Erasing Existing `balanceOf()` and Voting Power

0xmystery

Summary

A severe vulnerability in veRAACToken.sol allows users to entirely lose their existing locked data and voting power when calling lock() while already having an existing lock as soon as _lockState.createLock() is invoked.

Vulnerability Details

A user who has previously created a lock calls lock. When _lockState.createLock() is triggered, it does not ensure state.locks[user].exists != true prior to permitting the execution of the function.

LockManager.sol#L117-L143

function createLock(
LockState storage state,
address user,
uint256 amount,
uint256 duration
) internal returns (uint256 end) {
// Validation logic remains the same
if (state.minLockDuration != 0 && state.maxLockDuration != 0) {
if (duration < state.minLockDuration || duration > state.maxLockDuration)
revert InvalidLockDuration();
}
​
if (amount == 0) revert InvalidLockAmount();
​
end = block.timestamp + duration;
state.locks[user] = Lock({
amount: amount,
end: end,
exists: true
});
​
state.totalLocked += amount;
​
emit LockCreated(user, amount, end);
return end;
}

Sadly, state.locks[user] is assigned the new struct Lock where the inputted amount is replacing the existing amount.

Next, _votingState.calculateAndUpdatePower() is invoked to calculate bias, i.e. newPower, with the inputted amount that renders the existing voting power obsolete (erased).

veRAACToken.sol#L229-L241

// Calculate initial voting power
(int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(
msg.sender,
amount,
unlockTime
);
​
// Update checkpoints
uint256 newPower = uint256(uint128(bias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
​
// Mint veTokens
_mint(msg.sender, newPower);

Although newPower is minted to the user, when withdraw() is eventually callable, the function will burn all power minted but only userLock.amount, i.e. the last inputted amount of raacToken is transferred to the user.

Impact

The user lost the previously existing voting power and would need to sustain the loss of not getting transferred the previous RAAC tokens sent in to the contract.

Such discrepancy also affects all other contracts dependent of getVotingPower() and getTotalVotingPower(). For instance, when calculateing pending rewards for a user using time-weighted average in FeeCollector._calculatePendingRewards(), the pendingAmount returned will be smaller than intended due to a smaller userVotingPower associated now:

FeeCollector.sol#L474-L488

/**
* @dev Calculates pending rewards for a user using time-weighted average
* @param user Address of the user
* @return pendingAmount Amount of pending 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;
}

Tools Used

Manual

Recommendations

Consider making the following change:

LockManager.sol#L117-L122

function createLock(
LockState storage state,
address user,
uint256 amount,
uint256 duration
) internal returns (uint256 end) {
+ Lock storage lock = state.locks[user];
+ require(!lock.exists, "Lock already exists!");
Updates

Lead Judging Commences

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

veRAACToken::lock called multiple times, by the same user, leads to loss of funds

Support

FAQs

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

Give us feedback!