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

VestingFactor may be applied to the time before it has been set.

jsmi

Summary

The TempleGold.setVestingFactor() function updates the vestingFactor without updating the lastMintTimestamp. As a result, the new vestingFactor might be applied retroactively to a time period that should have used the old vestingFactor.

Vulnerability Details

The relevant code for TempleGold.setVestingFactor() is as follows:

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;
/// @dev initialize
@> if (lastMintTimestamp == 0) { lastMintTimestamp = uint32(block.timestamp); }
emit VestingFactorSet(_factor.numerator, _factor.denominator);
}

As shown above, the lastMintTimestamp is updated only when it is zero. Otherwise, it remains unchanged. Therefore, the newly set vestingFactor may apply to the time period [lastMintTimestamp, block.timestamp], which should have used the old vestingFactor.

Impact

The new vestingFactor might incorrectly apply to periods prior to its actual setting, leading to potential inconsistencies in vesting calculations.

Tools Used

Manual Review.

Recommendations

It's recommended to call the mint() function before updating the vestingFactor. This ensures that all vesting calculations are finalized using the old vestingFactor before it gets updated.

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(); }
+ mint();
vestingFactor = _factor;
/// @dev initialize
if (lastMintTimestamp == 0) { lastMintTimestamp = uint32(block.timestamp); }
emit VestingFactorSet(_factor.numerator, _factor.denominator);
}

Additionally, the visibility specifier of the mint() function should be changed from external to public.

By implementing this change, you ensure that the new vestingFactor is only applied to future time periods, maintaining the integrity of the vesting calculations.

Updates

Lead Judging Commences

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

When the operators call `TempleGold::setVestingFactor` to modify the `VestingFactor`, the tokens accumulated based on the previous factor are not resolved.

Support

FAQs

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