hats-finance / Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777

IBO, Vesting & Bond mecanism repo prepared for Hat finance audit competition
0 stars 0 forks source link

The owner may assign a **stale timestamp** into the `_startTimestamp` parameter due to lack of the input validation #68

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @0xmuxyz Submission hash (on-chain): 0x748b570d274f8096f221fe7657e7a5354de09a4b783c01494e0368ec645ec312 Severity: medium

Description: Title:\ The owner may assign a stale timestamp into the _startTimestamp parameter due to lack of the input validation

Severity:\ Medium

Description:\ When the owner create a new vesting schedule, the owner call the VestingCvg#createVestingSchedule().

Within the VestingCvg#createVestingSchedule(), the _startTimestamp would be assigned as a parameter. And then, it would be stored into the startTimestamp property of the vestingSchedules[nextVestingScheduleId] storage like this: VestingCvg.sol#L173\ VestingCvg.sol#L205

    /**
     * @notice Create vestingScheduleId for a specific type of presalers
                    - function only usable by owner
                    - update the total CVG amount available on this contract
     * @param _totalAmount total CVG amount allocated for the presaler type
     * @param _startTimestamp start timestamp of the vesting, every vesting should have the same start
     * @param _daysBeforeCliff daysBeforeCliff - period in days between start of the schedule and the cliff
     * @param _daysAfterCliff daysAfterCliff - period in days between the cliff and the end of the vesting
     * @param _vestingType type presaler vesting (ex : 1=SEED/PRESEED, 2=WL_S, 3=WL_M, 4=WL_L, 5=TEAM, 6=DAO)
     * @param _dropCliff Percent drop at the daysBeforeCliff release (per 1000) => 50% = 500 / 5% = 50
     */
    function createVestingSchedule(
        uint256 _totalAmount,
        uint184 _startTimestamp, ///<------- @audit
        uint16 _daysBeforeCliff,
        uint16 _daysAfterCliff,
        uint8 _vestingType,
        uint24 _dropCliff
    ) external onlyOwner {
        ...
        uint256 vestingScheduleId = nextVestingScheduleId;
        vestingSchedulesIds.push(vestingScheduleId);

        vestingSchedulesTotalAmount += _totalAmount;

        //set struct vesting
        vestingSchedules[nextVestingScheduleId] = VestingSchedule({
            revoked: false,
            totalAmount: _totalAmount,
            totalReleased: 0,
            startTimestamp: _startTimestamp, ///<--------- @audit 
            daysBeforeCliff: _daysBeforeCliff,
            daysAfterCliff: _daysAfterCliff,
            vestingType: _vestingType,
            dropCliff: _dropCliff
        });
        ...

A timestamp, which is assigned into the _startTimestamp parameter when the VestingCvg#createVestingSchedule() wold be called above, is supposed to be more advanced than the current timestamp (block.timestamp).

However, there is no input validation to check wether or not a timestamp, which is assigned into the _startTimestamp parameter, would be more advanced than the current timestamp (block.timestamp).

This lead to that the owner accidentally assign an stale timestamp into the _startTimestamp parameter and it would be stored into the startTimestamp property of the vestingSchedules[nextVestingScheduleId] storage when the owner call the VestingCvg#createVestingSchedule() above.

Recommendation:\ Within the VestingCvg#createVestingSchedule(), consider adding an input validation to check wether or not a timestamp, which is assigned into the _startTimestamp parameter, would be more advanced than the current timestamp (block.timestamp) like this:

    function createVestingSchedule(
        uint256 _totalAmount,
        uint184 _startTimestamp, 
        uint16 _daysBeforeCliff,
        uint16 _daysAfterCliff,
        uint8 _vestingType,
        uint24 _dropCliff
    ) external onlyOwner {
        ...
+       require(_startTimestamp > block.timestamp, "_startTimestamp must be more advanced than the current timestamp");

        //set struct vesting
        vestingSchedules[nextVestingScheduleId] = VestingSchedule({
            revoked: false,
            totalAmount: _totalAmount,
            totalReleased: 0,
            startTimestamp: _startTimestamp,
            daysBeforeCliff: _daysBeforeCliff,
            daysAfterCliff: _daysAfterCliff,
            vestingType: _vestingType,
            dropCliff: _dropCliff
        });
        ...
walk-on-me commented 1 year ago

Hello, Thanks a lot for your attention.

This issue is a configuration issue, we'll set up manually those timestamps by hand and verifiy that they are the same.

We have so to consider this issue as Invalid