TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

setting a new vesting factor will change all unclaimed yield

pyro

Summary

Setting a new vesting factor with setVestingFactor will change all unclaimed yield.

Vulnerability Details

Although setVestingFactor is intended as a "just in case" function for major changes, it is still a high priority for the audit. Calling this function will change the vesting factor but will only set lastMintTimestamp if its value was 0 before.

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L130

function setVestingFactor(VestingFactor calldata _factor) external override onlyOwner {
if (_factor.numerator == 0 || _factor.denominator == 0) {
revert CommonEventsAndErrors.ExpectedNonZero();
}
​
if (_factor.numerator > _factor.denominator) {
revert CommonEventsAndErrors.InvalidParam();
}
vestingFactor = _factor;
​
// Will not set it during a midway change
if (lastMintTimestamp == 0) {
lastMintTimestamp = uint32(block.timestamp);
}
emit VestingFactorSet(_factor.numerator, _factor.denominator);
}

The issue is that any unclaimed yield will change with the vesting factor. Yield is calculated based on the last claimed time and the current vesting factor.

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L253

// timeDiff * MAX_SUPPLY * numerator / denominator
mintAmount = TempleMath.mulDivRound(
(block.timestamp - _lastMintTimestamp) * (MAX_SUPPLY),
vestingFactorCache.numerator,
vestingFactorCache.denominator,
false
);

Example:

  1. The vesting factor distributes 0.96% per week (set for 2 years).

  2. After 3 months, setVestingFactor is called, changing the distribution to 1 year (1.92% per week).

  3. Yield was not claimed in 1 week due to inactivity or not reaching MINIMUM_MINT.

  4. After the change, the new unclaimed yield instantly doubles from 0.96% to 1.92% as the distribution period shortens.

In the above example, changing the vesting factor without minting first will instantly double our yield. In simple terms it will make the generated yield time claim at a higher rate

Impact

There could be a loss or gain for users and the protocol, depending on whether the factor increases or decreases.

Tools Used

Manual review

Recommendations

Check if _canDistribute is true and call mint before changing the vesting factor. This will also set lastMintTimestamp.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Changes to vesting period is not handled inside `_getVestingRate`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.