bitsurfer - `vestingStart` can be set to far back date, less than block.timestamp, resulting locked, claimable, vested amount manipulation

[sherlock-admin2]

bitsurfer

high

vestingStart can be set to far back date, less than block.timestamp, resulting locked, claimable, vested amount manipulation

Summary

vestingStart can be set to back date, which is less than block.timestamp resulting locked, claimable, vested amount manipulation

Vulnerability Detail

inside deployVestingContract, there is no check if vestingStart should be be in the future date (greater than block.timestamp)

This means, user can input back-date, pretends that they already vest long time in the past.

This can manipulate the locked, claimable, vested amount of token of that user, which can break the vesting mechanism so is not what expected.

File: VestingEscrowFactory.sol
51:     function deployVestingContract(
52:         uint256 amount,
53:         address recipient,
54:         uint40 vestingDuration,
55:         uint40 vestingStart,
56:         uint40 cliffLength,
57:         bool isFullyRevokable,
58:         bytes calldata initialDelegateParams
59:     ) external returns (address escrow) {
60:         if (vestingDuration == 0) revert INVALID_VESTING_DURATION();
61:         if (cliffLength > vestingDuration) revert INVALID_VESTING_CLIFF();
62:         if (recipient == address(0)) revert INVALID_RECIPIENT();
63:         if (amount == 0) revert INVALID_AMOUNT();
64:
...
75:     }

Impact

User can manipulate to make they already vesting in a long time, manipulate the locked, claimable, vested amount thus taking advantage of it

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrowFactory.sol#L51-L75

Tool used

Manual Review

Recommendation

Consider to add check to make sure vestingStart >= block.timestamp if (vestingStart < block.timestamp) revert INVALID_VESTING_START();

Duplicate of #101

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

pratraut commented:

'valid as check on a start time is must to prevent owner deploying vesting contract with start time from past'