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

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

Team and DAO vesting is flawed #27

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

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

Github username: @iamjakethehuman Submission hash (on-chain): 0x929b217c23a1a61809b6aef87d3d303f0948b11c55414746a1f91b6b027c6ae8 Severity: medium

Description: Description\ Team and DAO vesting is flawed

Attack Scenario\ In VestingCvg different types of vesting schedules can be created, including Team and DAO vesting. However, the Team and DAO vestings are flawed.

   function releaseTeamOrDao(bool _isTeam) external {
        (uint256 amountToRelease, uint256 vestingScheduleId) = _computeReleaseAmountTeamDao(_isTeam);

        require(amountToRelease > 0, "NOT_RELEASABLE");
        require(!vestingSchedules[vestingScheduleId].revoked, "VESTING_REVOKED");

        if (_isTeam) {
            require(msg.sender == whitelistedTeam, "NOT_TEAM");
            amountReleasedTeam += amountToRelease;
        } else {
            require(msg.sender == whitelistedDao, "NOT_DAO");
            amountReleasedDao += amountToRelease;
        }

        /// @dev update totalReleased & amountReleasedIdSeed & vestingSchedulesTotalAmount
        vestingSchedules[vestingScheduleId].totalReleased += amountToRelease;

        vestingSchedulesTotalAmount -= amountToRelease;

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

Upon releasing Team or Dao, there is a state variable amountReleasedTeam/ amountReleasedDao which keeps track of how much amount is released. This value is later used in _computeReleaseAmountTeamDao in order to calculate the amountToRelease

amountToRelease =
                    amountDroppedAtCliff +
                    (((ONE_GWEI - ratio) * totalAmountAfterCliff) / ONE_GWEI) -
                    totalAmountReleased; //@audit: totalAmountReleased = amountReleasedTeam /amountReleasedDao 

Upon revoking a Team/Dao vesting, these values are not cleared. Overall the logic creates the following problems:

  1. After 1 vesting schedule is created and finishes, no more can be created as in calculateRelease totalAmount will always equal totalAmountReleased
  2. If a DAO/ Team vesting has began and is revoked after X time and then a new vesting schedule is created with the same parameters, no amount will be able to be released for the first X time after the creation due to revert in calculateRelease due to underflow.
                amountToRelease =
                    amountDroppedAtCliff +
                    (((ONE_GWEI - ratio) * totalAmountAfterCliff) / ONE_GWEI) -
                    totalAmountReleased; //@audit totalAmountReleased will be larger than amountDroppedAtCliff +
                                                          // (((ONE_GWEI - ratio) * totalAmountAfterCliff) / ONE_GWEI)
                                                          // for the first X time
                                                          // hence the transactions will revert due to underflow

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) After revoking a TEAM/Dao vesting, clear the value of the respective amountReleasedTeam/ amountReleasedDao

0xR3vert commented 1 year ago

Hello, Thanks a lot for your attention. The revoke function is not intended to be used (only in case of extreme emergency). In conclusion we have so to consider this issue as invalid.