totalLocked value can be released immediately after creation
Summary
In the deployVestingContract function from the VestingEscrowFactory contract, we see that less attention was paid to the vestingStart which allows a user to be able to unlock the entire tokens vested almost immediately after creation if a mistake was made while calling the function. The deployVestingContract can be called by anyone in or outside the Rio organization, so this mistake is highly likely to happen
Vulnerability Detail
If the vestingStart parameter was less than the block.timestamp, say 100 or 100000, and the vestingDuration is roughly a day, let's say 100000, this means we have endTime as 100100 in scenario 1 and 200000 in scenario 2 during deployment of the VestingEscrow contract.
The recipient can then call the claim function with max Value of the tokens locked, the claim function checks for claimable tokens using the unclaimed function .
In the unclaimed function the claimTime is now, and totalClaimed is zero. The _totalVestedAt function is called with the claimTime.
The math checks _totalLocked * (claimTime - startTime) / (endTime - startTime).
In scenario 1, this gives _totalLocked * (claimTime / 100100), using the block.timestamp of right now, the claimTime is 1705146362, so _totalVestedAt returns a minimum of _totalLocked and approx 17000 * _totalLocked, which is _totalLocked, allowing the recipient to claim entire tokens almost immediately.
In scenario 2, it is a minimum of _totalLocked and approx 9000 * _totalLocked, which still causes the same issue
Impact
This invalidates the true purpose of the smart contract and could cause the users to lose trust in the protocol as the vesting features do not work in some cases
John_Femi
medium
totalLocked value can be released immediately after creation
Summary
In the
deployVestingContract
function from the VestingEscrowFactory contract, we see that less attention was paid to the vestingStart which allows a user to be able to unlock the entire tokens vested almost immediately after creation if a mistake was made while calling the function. ThedeployVestingContract
can be called by anyone in or outside the Rio organization, so this mistake is highly likely to happenVulnerability Detail
If the
vestingStart
parameter was less than theblock.timestamp
, say 100 or 100000, and thevestingDuration
is roughly a day, let's say 100000, this means we have endTime as 100100 in scenario 1 and 200000 in scenario 2 during deployment of the VestingEscrow contract.The recipient can then call the
claim
function with max Value of the tokens locked, the claim function checks for claimable tokens using theunclaimed
function .In the
unclaimed
function theclaimTime
is now, andtotalClaimed
is zero. The_totalVestedAt
function is called with theclaimTime
.The math checks
_totalLocked * (claimTime - startTime) / (endTime - startTime).
In scenario 1, this gives
_totalLocked * (claimTime / 100100)
, using theblock.timestamp
of right now, theclaimTime
is 1705146362, so_totalVestedAt
returns a minimum of_totalLocked
and approx17000 * _totalLocked
, which is_totalLocked
, allowing the recipient to claim entire tokens almost immediately.In scenario 2, it is a minimum of
_totalLocked
and approx9000 * _totalLocked
, which still causes the same issueImpact
This invalidates the true purpose of the smart contract and could cause the users to lose trust in the protocol as the vesting features do not work in some cases
Code Snippet
https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrowFactory.sol#L55
https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L137
https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L123
https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L274
Tool used
Manual Review
Recommendation
Add a check to force
vestingStart
to be at leastblock.timestamp
Duplicate of #101