Core Contracts

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

Users can invoke `lock` function multiple times, overriding their current state

Summary

The veRAACToken::lock function has no limit to how many times it can be called. When a user calls it each subsequent time, the users' Lock gets overridden. When they call the withdraw function after the duration, they lose all of their previous locked amount.

Vulnerability Details

The veRAACToken contract contains a critical vulnerability in its lock mechanism that leads to permanent token loss when users create multiple lock positions. The core issue lies in how the contract handles subsequent calls to the lock() function:

// In veRAACToken.sol
function lock(uint256 amount, uint256 duration) external nonReentrant whenNotPaused {
// ... validation checks ...
// Overwrites existing lock without handling previous locked tokens
_lockState.createLock(msg.sender, amount, duration);
}
// In LockManager.sol
function createLock(...) internal returns (uint256 end) {
// @audit-issue Overrides the `Lock` struct.
state.locks[user] = Lock({
amount: amount, // Only stores new amount
end: end,
exists: true
});
}

When a user calls lock() for the second time, the following occurs:

  1. The previous lock position is overwritten in the lock mapping

  2. The record of the initially locked tokens is lost

  3. The tokens from the first lock remain in the contract but become irretrievable

Example scenario demonstrating the vulnerability:

  1. Bob calls the lock function with 500 tokens for 400 days

    • Lock state: amount = 500, end = timestamp + 400 days

  2. After some time passes Bob calls the lock function again with 300 tokens, for 750 days

    • Lock state: amount = 300, end = timestamp + 750 days

  3. When the 750 days pass, Bob calls the withdraw function which only withdraws the 300 tokens and the previous 500 tokens are permanently locked, because of the state overriding.

Impact

The user loses all of his previous locked amounts

PoC

This test demonstrates the scenario in Vulnerability Details.

Foundry Test Used:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import {Test, console} from "forge-std/Test.sol";
import {RAACToken} from "../../../src/RAAC-Protocol/veRAACToken/RAACToken.sol";
import {veRAACToken} from "../../../src/RAAC-Protocol/veRAACToken/veRAACToken.sol";
import {IveRAACToken} from "../../../src/RAAC-Protocol/veRAACToken/interfaces/IveRAACToken.sol";
contract TestVeRAACToken is Test {
RAACToken public _raacToken;
veRAACToken public _veRAACToken;
address deployer = makeAddr("deployer");
address beneficiary = makeAddr("beneficiary");
address userOne = makeAddr("userOne");
address userTwo = makeAddr("userTwo");
address Bob = makeAddr("Bob");
uint256 initialTax = 500;
function setUp() public {
vm.startPrank(deployer);
// RAAC TOKEN
_raacToken = new RAACToken(deployer, initialTax, initialTax);
_raacToken.setMinter(deployer);
_raacToken.mint(address(beneficiary), 1000 ether);
_raacToken.mint(address(userOne), 1000 ether);
_raacToken.mint(address(Bob), 1000 ether);
// VERAAC TOKEN
_veRAACToken = new veRAACToken(address(_raacToken));
vm.stopPrank();
vm.prank(beneficiary);
_raacToken.approve(address(_veRAACToken), type(uint256).max);
vm.prank(Bob);
_raacToken.approve(address(_veRAACToken), type(uint256).max);
vm.prank(userOne);
_raacToken.approve(address(_veRAACToken), type(uint256).max);
vm.prank(deployer);
_raacToken.manageWhitelist(address(_veRAACToken), true);
}
function testLockingTwoTimeVulnerability() public {
uint256 raacTokenInitial = _raacToken.balanceOf(Bob);
uint256 duration = 400 days;
uint256 amount = 500 ether;
vm.startPrank(Bob);
_veRAACToken.lock(amount, duration);
vm.stopPrank();
vm.warp(block.timestamp + 50 days);
vm.roll(block.timestamp + 50 * 12000);
uint256 duration2 = 750 days;
uint256 amount2 = 300 ether;
vm.startPrank(Bob);
_veRAACToken.lock(amount2, duration2);
vm.stopPrank();
vm.warp(block.timestamp + 751 days);
vm.roll(block.timestamp + 751 * 12000);
uint256 raacTokenBefore = _raacToken.balanceOf(Bob);
console.log("Bob RAAC Token Balance Before:", raacTokenBefore);
vm.startPrank(Bob);
_veRAACToken.withdraw();
vm.stopPrank();
uint256 raacTokenAfter = _raacToken.balanceOf(Bob);
console.log("Bob RAAC Token Balance After:", raacTokenAfter);
// Asserts Bob did not withdraw all of his locked tokens after the period passed
assert(raacTokenInitial > raacTokenAfter);
}
}

Tools Used

Manual Review, Foundry Test

Recommendations

Make sure users can call the lock function only when their previous lock has expired and they have withdrawn their tokens.

  1. Add a mapping tracking if a user has an active lock

+ mapping(address locker => bool hasActiveLock) hasActiveLock;
function lock(uint256 amount, uint256 duration) external nonReentrant whenNotPaused {
+ if(hasActiveLock[msg.sender]) revert();
...the rest of the code...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 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.