sherlock-audit / 2024-01-rio-vesting-escrow-judging

3 stars 2 forks source link

slowfi - Add min/max boundaries to vesting time #82

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

slowfi

medium

Add min/max boundaries to vesting time

Summary

The vesting time start and duration are not sanitised properly and can lead to misbehaves.

Vulnerability Detail

The deployVestingContract function from the VestingEscrowFactory does not perform enough checks on the input data.

The vestingStart parameter should be at least equal to the block.timestamp to avoid creating an already passed vesting time. Or at least that vestingStart plus vestingDuration is bigger than the actual timestamp. If not is possible to create already passed vesting contracts from where users are able to retrieve tokens directly, which does not make sense for the usage of these contracts.

Also a max boundary can be considered to avoid creating a vesting for more time than expected. However this case is less important as revoke functions exist.

Impact

Creating an already passed vested time is inconsistent and can lead to token retrieval before than expected.

Code Snippet

VestingEscrowFactory.sol#L65-L68

Tool used

Manual Review

Recommendation

Consider adding at least a minimum boundary check comparing against the current timestamp to ensure consistency and proper functionality.

Duplicate of #101