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

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

Creating a new vesting schedule for a type overwrites the old one #18

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

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

Github username: @8ahoz Submission hash (on-chain): 0x6f0d9a78a92ea6b22f74dc3a118e49f8bddce6fc94210dcaa499e02dd16d4d8f Severity: medium

Description: Description:

createVestingSchedule() in VestingCvg.sol creates a new Vesting schedule and sets the vesting type to the newly created schedule.

vestingIdForType[_vestingType] = nextVestingScheduleId;

However by doing so, it does not check if there is already a vesting schedule for that vestingType and directly resets to a new one.

This may cause two problems with different impacts depending on what is the intended behavior:

  1. If updating to a new schedule is not intended, this means the old schedule is lost and the users now will claim their tokens according to the new vesting schedule. This is a medium severity issue causing unexpected changes in the users locking periods
  2. If this is intended, it might be ok considering the admins may want to use this to update any current vesting schedule. However doing so does not revoke the old vesting schedule as it should. This causes vestingSchedulesTotalAmount to become higher than it should which affects the returned amount from withdrawOnlyExcess(). This is a Lower severity issue

Recommended Mitigation:

Check if the vestingIdForType[_vestingType] exists, depending on the intended behavior, do one of the followings:

  1. Revert if an update to vesting schedules not intended. Or ->
  2. Call revokeVestingSchedule() with the old vestingId
0xR3vert commented 1 year ago

Hello, Thanks a lot for your attention. We don't consider misconfiguration as an issue. We are aware of this and assume the risks. The revoke is here just in case of extreme emergency. In conclusion we have so to consider this issue as invalid.