Winning Currant Koala - Incorrect `totalVested` Adjustment In `_resetVestingPlans`

by ADMIN 84 views

Winning Currant Koala - Incorrect totalVested Adjustment in _resetVestingPlans

Introduction

In the world of blockchain and cryptocurrency, the concept of vesting plans is crucial for managing token allocations and ensuring fair distribution among users. However, a critical issue has been discovered in the _resetVestingPlans function, which can lead to incorrect adjustments in the totalVested value. This article will delve into the root cause of the problem, its impact on the protocol, and propose a mitigation strategy to rectify the issue.

Summary

The _resetVestingPlans function is responsible for updating the totalVested value by subtracting the old locked amount and adding the new total. However, this approach leads to an incorrect, often inflated, overall allocation for the token. The root cause lies in the function capturing oldTotal as vestingPlan.lockedAmount() instead of the vesting plan's previous total allocation.

Root Cause

The root cause of the issue is the incorrect calculation of oldTotal in the _resetVestingPlans function. Instead of using the vesting plan's previous total allocation, the function uses the locked amount, which is a partial value. When resetting, subtracting this partial value and adding the new total results in miscalculation.

Internal Pre-conditions

For the _resetVestingPlans function to be executed, the following pre-conditions must be met:

  • A vesting plan with a defined total allocation exists.
  • The vesting plan has already unlocked a portion, so lockedAmount() is lower than the total.

External Pre-conditions

The following external pre-conditions must be satisfied for the _resetVestingPlans function to be called:

  • Users have an existing vesting schedule that is being modified.

Attack Path

The attack path involves calling the resetVestingPlans function with new amounts. For each user, the function subtracts vestingPlan.lockedAmount() (e.g., the locked portion) instead of the entire previous allocation, then adds the new total.

Impact

The protocol's accounting becomes inconsistent. Future calculations that depend on totalVested may be wrong, potentially affecting vesting, claims, and overall token economics.

Proof of Concept (PoC)

To illustrate the issue, consider the following example:

  • A vesting plan is set up with 1000 tokens.
  • When resetting it to 600 tokens, the totalVested value should be updated as follows:
    • Subtract the old total (1000) from the totalVested value.
    • Add the new total (600) to the totalVested value.
    • The resulting totalVested value should be 600.

However, the code does the following:

  • oldTotal is set to vestingPlan.lockedAmount() (400), which is the locked portion of the original 1000 tokens.
  • totalVested is updated by subtracting oldTotal (400) and adding the new total (600).
  • The resulting totalVested value is 200, which is incorrect.

The problem is that the code is using the old locked amount (400) instead of the old total (1000). To fix this, the line totalVested[token] = totalVested[token] - oldTotal + amount; should be updated to use the correct oldTotal value.

Mitigation

To mitigate this issue, the oldTotal value should be calculated as the sum of the locked amount and the unlocked amount. This can be achieved by adding the following line to the _resetVestingPlans function:

uint256 oldTotal = vestingPlan.lockedAmount() + vestingPlan.unlockedAmount();

By making this change, the totalVested value will be updated correctly, ensuring that the protocol's accounting remains consistent.

Conclusion

In conclusion, the _resetVestingPlans function contains a critical issue that can lead to incorrect adjustments in the totalVested value. This issue can have a significant impact on the protocol's accounting and may affect vesting, claims, and overall token economics. By understanding the root cause of the problem and implementing the proposed mitigation strategy, the protocol can ensure that the totalVested value is updated correctly, maintaining the integrity of the token allocation system.
Winning Currant Koala - Incorrect totalVested Adjustment in _resetVestingPlans

Q&A

Q: What is the root cause of the issue in the _resetVestingPlans function?

A: The root cause of the issue is the incorrect calculation of oldTotal in the _resetVestingPlans function. Instead of using the vesting plan's previous total allocation, the function uses the locked amount, which is a partial value.

Q: What are the internal pre-conditions for the _resetVestingPlans function to be executed?

A: The internal pre-conditions for the _resetVestingPlans function to be executed are:

  • A vesting plan with a defined total allocation exists.
  • The vesting plan has already unlocked a portion, so lockedAmount() is lower than the total.

Q: What are the external pre-conditions for the _resetVestingPlans function to be called?

A: The external pre-conditions for the _resetVestingPlans function to be called are:

  • Users have an existing vesting schedule that is being modified.

Q: What is the impact of the incorrect totalVested adjustment on the protocol?

A: The protocol's accounting becomes inconsistent. Future calculations that depend on totalVested may be wrong, potentially affecting vesting, claims, and overall token economics.

Q: Can you provide an example to illustrate the issue?

A: Consider the following example:

  • A vesting plan is set up with 1000 tokens.
  • When resetting it to 600 tokens, the totalVested value should be updated as follows:
    • Subtract the old total (1000) from the totalVested value.
    • Add the new total (600) to the totalVested value.
    • The resulting totalVested value should be 600.

However, the code does the following:

  • oldTotal is set to vestingPlan.lockedAmount() (400), which is the locked portion of the original 1000 tokens.
  • totalVested is updated by subtracting oldTotal (400) and adding the new total (600).
  • The resulting totalVested value is 200, which is incorrect.

Q: How can the issue be mitigated?

A: To mitigate this issue, the oldTotal value should be calculated as the sum of the locked amount and the unlocked amount. This can be achieved by adding the following line to the _resetVestingPlans function:

uint256 oldTotal = vestingPlan.lockedAmount() + vestingPlan.unlockedAmount();

Q: What is the correct approach to updating the totalVested value?

A: The correct approach to updating the totalVested value is to subtract the old total and add the new total. This can be achieved by using the following formula:

totalVested = totalVested - oldTotal + amount;

Where oldTotal is the sum of the locked amount and the unlocked amount, and amount is the new total.

Q: Why is it important to update the totalVested value correctly?

A: Updating the totalVested value correctly is important because it ensures that the protocol's accounting remains consistent. If the totalVested value is not updated correctly, it can lead to incorrect calculations and potentially affect vesting, claims, and overall token economics.