Seeds can release more tokens than anticipated.

[hats-bug-reporter[bot]]

Github username: @iamjakethehuman Submission hash (on-chain): 0x6b9017b050deeb8b397463e112b2ee2eab9c1d4298cf90b2050055e82b2461ac Severity: high

Description: Description\ Seeds can release more tokens than anticipated.

Attack Scenario\ Via VestingCvg vestingschedules can be created (e.g. for a seed). By doing so an amount to be released is specified.

        vestingSchedules[nextVestingScheduleId] = VestingSchedule({
            revoked: false,
            totalAmount: _totalAmount, //@audit - totalAmount is the anticipated limit of tokens that should be able to be released
            totalReleased: 0,
            startTimestamp: _startTimestamp,
            daysBeforeCliff: _daysBeforeCliff,
            daysAfterCliff: _daysAfterCliff,
            vestingType: _vestingType,
            dropCliff: _dropCliff
        });

However, due to a flaw in logic seeds can bypass this restrictions and release infinite tokens. Upon calling realeseSeed a call to _computeReleaseAmount is made. It calculates the needed amountToRelease.

    function releaseSeed(uint256 _tokenId) external onlyOwnerOfSeed(_tokenId) {
        (uint256 amountToRelease, , , uint256 vestingScheduleId) = _computeReleaseAmount(_tokenId, TYPE_SEED);
        require(amountToRelease > 0, "NOT_RELEASABLE");
        require(!vestingSchedules[vestingScheduleId].revoked, "VESTING_REVOKED");

        //update totalReleased & amountReleasedId & vestingSchedulesTotalAmount
        vestingSchedules[vestingScheduleId].totalReleased += amountToRelease;

        amountReleasedIdSeed[_tokenId] += amountToRelease;

        vestingSchedulesTotalAmount -= amountToRelease;

        //transfer Cvg amount to release
        cvg.transfer(msg.sender, amountToRelease);
    }

However, there isn't a check to make sure the new vestingSchedules[vestingScheduleId].totalReleased isnt > vestingSchedules[vestingScheduleId].totalAmount. This would allow for seed owners to release tokens endlessly and bypass restrictions. Furthermore, once vestingSchedules[vestingScheduleId].totalReleased > vestingSchedules[vestingScheduleId].totalAmount, the vesting schedule cannot even be revoked as a call to revokeVestingSchedulewill revert due to underflow

    function revokeVestingSchedule(uint256 _vestingScheduleId) external onlyOwner {
        require(!vestingSchedules[_vestingScheduleId].revoked, "IRREVOCABLE");
        uint256 totalAmountScheduleId = vestingSchedules[_vestingScheduleId].totalAmount;
        uint256 releasedAmountScheduleId = vestingSchedules[_vestingScheduleId].totalReleased;
        uint256 unreleasedAmountScheduleId = totalAmountScheduleId - releasedAmountScheduleId; //@audit - underflow here 
        vestingSchedulesTotalAmount -= unreleasedAmountScheduleId;
        vestingSchedules[_vestingScheduleId].revoked = true;
    }

This way seed owners can bypass all restrictions, release more tokens than anticipated and break accounting and core functionalities, hence the High severity.

Note: this issue is also true for WL and Ibo owners.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional) Add a restriction that the released vestingSchedules[vestingScheduleId].totalReleased <= vestingSchedules[vestingScheduleId].totalAmount

[0xR3vert]

Hello, Thanks a lot for your attention. For a given vestingSchedule the totalAmount will not change (unless if it's revoked). Also the total released (for a vestingSchedule) and the amountReleased (for a tokenId) are always calculated in regards of the totalAmount. And finally we increment theses variables at the end of the function. If totalAmount is reached, the transaction will revert with the following "NOT_RELEASABLE" require (because the amount calculated will be 0). Same for the revoke (that will only be used in case of extreme emergency), the underflow is not possible because the totalReleased will always be lower or equal to totalAmount. In conclusion we have so to consider this issue as invalid.