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

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

`calculateRelease` logic in `VestingCvg.sol` is flawed #33

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

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

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

Description: Description\ The calculateRelease logic in VestingCVG.sol is flawed.

Attack Scenario\ Let's look at the current implementation of calculateRelease

    function calculateRelease(
        uint256 vestingSchedulesId,
        uint256 totalAmount,
        uint256 totalAmountReleased
    ) private view returns (uint256 amountToRelease) {
        uint256 cliffTimestamp = vestingSchedules[vestingSchedulesId].startTimestamp +
            vestingSchedules[vestingSchedulesId].daysBeforeCliff *
            ONE_DAY;

        uint256 endVestingTimestamp = cliffTimestamp + vestingSchedules[vestingSchedulesId].daysAfterCliff * ONE_DAY;

        if (block.timestamp > cliffTimestamp) {
            if (block.timestamp > endVestingTimestamp) {
                amountToRelease = totalAmount - totalAmountReleased;
            } else {
                uint256 ratio = ((endVestingTimestamp - block.timestamp) * ONE_GWEI) /
                    (endVestingTimestamp - cliffTimestamp);

                uint256 amountDroppedAtCliff = (totalAmount * vestingSchedules[vestingSchedulesId].dropCliff) / 1000;

                uint256 totalAmountAfterCliff = totalAmount - amountDroppedAtCliff;

                amountToRelease =
                    amountDroppedAtCliff +
                    (((ONE_GWEI - ratio) * totalAmountAfterCliff) / ONE_GWEI) -  // @audit - this line is problematic 
                    totalAmountReleased;
            }
        }
    }

When the whole time hasn't passed, we enter the else statement. Ratio calculates what percentage after cliff has passed. However, when calculating the amountToRelease, instead of releasing the X% of tokens relative to the % of time of the cliff has passed, we release 100 - X. Meaning that after 1% of the cliff time has passed, we will be able to release 99% of the totalAmountAfterCliff. Same logic will apply that after 99% of the time has passed we will only be able to release 1% of the totalAmountAfterCliff.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) Instead of subtracting ratio from ONE_GWEI, just multiply the totalAmountAfterCliff by ratio.

deadrosesxyz commented 1 year ago

Yep, mistake on my part, this issue is actually invalid

0xR3vert commented 1 year ago

Hello, Thanks a lot for your attention and your participation !