0xGreyWolf - Missing `block.timestamp` validation check for parameter `vestingStart` in `VestingEscrowFactory::deployVestingContract()` will deploy an inutile contract.

[sherlock-admin]

0xGreyWolf

medium

Missing block.timestamp validation check for parameter vestingStart in VestingEscrowFactory::deployVestingContract() will deploy an inutile contract.

Summary

• In VestingEscrowFactory::deployVestingContract(), this parametervestingStart should have a validity check where the value should be at least more than the block.timestamp.

Vulnerability Detail

• If by mistake, the value passed is less than block.timestamp, then at a certain point in succeeding calls, the arithmetic will be wrong and functions will revert. Here's the code VestingEscrowFactory::deployVestingContract() for quick reference.

    /// @notice Deploy and fund a new vesting contract.
    /// @param amount The amount of tokens to lock in the vesting contract.
    /// @param recipient The recipient of the vesting contract.
    /// @param vestingDuration The duration of the vesting contract.
    /// @param vestingStart The start of the vesting contract.
    /// @param cliffLength The cliff length of the vesting contract.
    /// @param isFullyRevokable Whether the vesting contract is fully revokable.
    /// @param initialDelegateParams The optional initial delegate information (skipped if empty bytes).
    function deployVestingContract(
        uint256 amount,
        address recipient,
        uint40 vestingDuration,
        uint40 vestingStart,
        uint40 cliffLength,
        bool isFullyRevokable,
        bytes calldata initialDelegateParams
    ) external returns (address escrow) {
        if (vestingDuration == 0) revert INVALID_VESTING_DURATION();
        if (cliffLength > vestingDuration) revert INVALID_VESTING_CLIFF();
        if (recipient == address(0)) revert INVALID_RECIPIENT();
        if (amount == 0) revert INVALID_AMOUNT();

        escrow = vestingEscrowImpl.clone(
            abi.encodePacked(
                address(this), token, recipient, vestingStart, vestingStart + vestingDuration, cliffLength, amount
            )
        );

        IERC20(token).safeTransferFrom(msg.sender, escrow, amount);
        IVestingEscrow(escrow).initialize(isFullyRevokable, initialDelegateParams);

        emit VestingEscrowCreated(msg.sender, recipient, escrow);
    }

Now here's the calculation that will be affected if the vestingStart value is wrong. It's in VestingEscrow::_totalVestedAt().

    /// @dev Returns the vested token amount at a specific time.
    /// @param time The time to retrieve the vesting amount for.
    function _totalVestedAt(uint256 time) internal pure returns (uint256) {
        uint40 _startTime = startTime();
        uint256 _totalLocked = totalLocked();
        if (time < _startTime + cliffLength()) {
            return 0;
        }
        return Math.min(_totalLocked * (time - _startTime) / (endTime() - _startTime), _totalLocked);
    }

Impact

Deployment of an inutile or useless contract

Code Snippet

VestingEscrowFactory::deployVestingContract()

Tool used

Manual Review

Recommendation

Add validation such that if vestingStart is less than block.timestamp, it should revert and declare the custom error too.

function deployVestingContract(
        uint256 amount,
        address recipient,
        uint40 vestingDuration,
        uint40 vestingStart,
        uint40 cliffLength,
        bool isFullyRevokable,
        bytes calldata initialDelegateParams
    ) external returns (address escrow) {        
        if (vestingDuration == 0) revert INVALID_VESTING_DURATION();
++      if (vestingStart < block.timestamp) revert INVALID_VESTING_START();        
        if (cliffLength > vestingDuration) revert INVALID_VESTING_CLIFF();
        if (recipient == address(0)) revert INVALID_RECIPIENT();
        if (amount == 0) revert INVALID_AMOUNT();
        ...
    {

Duplicate of #101

[solimander]

Considering invalid.

Admin incorrectly enters an input parameter.