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

updating vestingFactor will also change the vestingFactor after the last update

cryptomoon

Summary

In contract TempleGold, vestingFactoris used to control the amount of tokens minted per second. The parameter vestingFactorcan be set or updated by using setVestingFactorfunction. When vestingFactoris updated, the new factor should be effective from the time it's changed. In current implementation, it is effective from lastMintTimestampwhich is not the expected behaviour.

Vulnerability Details

When the vestingFactorparameter is updated using setVestingFactorfunction, it should only be effective from time at which it's updated. However, in current implementation of code, lastMintTimestampcan be different from block.timestampat which vestingFactoris updated. So, new vestingFactorwill be effective from lastMintTimestampand not the block.timestampwhen setVestingFactorcalled. lastMintTimestampis updated when mintfunction is called.

Steps to reproduce:

1) ownersets vestingFactor by calling setVestingFactorfunction for first time. For sake of understanding, let's assume it's factor.numeratoris 2 and factor.denominatoris 100. Let's assume MAX_SUPPLYis 100. So, it would take 50seconds to reach MAX_SUPPLY.

2) at time = 5seconds, usercalls mint. It mints 10tokens. lastMintTimestampis updated to 5.

3) at time = 25seconds, ownerupdates the vestingFactorby calling setVestingFactorfunction. let's assume new factor.numeratoris 1 and factor.denominatoris 100

4) at time = 30seconds, usercalls mint. It mints (30 - 5) * 1 = 25 tokens. But that's not correct. The number tokens that should be minted is (25 - 5) * 2 + (30 - 25) * 1 = 45 tokens. The main reason for this is new vestingFactorshould be effective from 25seconds and not 5seconds.

Impact

Updating vestingFactor is not effective from timestampat which it's updated but it's effective from lastMintTimestamp. This can result in incorrect minting of tokens.

Tools Used

Manual Review

Recommendations

ensure that mintwas called in the same block as setVestingFactor.

This can be done by calling mintinternally in the setVestingFactorfunction or adding following condition in setVestingFactorfunction.

function setVestingFactor(VestingFactor calldata _factor) external override onlyOwner { //todo updating vestingFactor will also check vesting factor of previous timestamp
if (_factor.numerator == 0 || _factor.denominator == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (_factor.numerator > _factor.denominator) { revert CommonEventsAndErrors.InvalidParam(); }
vestingFactor = _factor;
// ADD FOLLOWING CONDITION
if (lastMintTimestamp != 0 && lastMintTimestamp != block.timestamp) {revert()};
/// @dev initialize
if (lastMintTimestamp == 0) { lastMintTimestamp = uint32(block.timestamp); }
emit VestingFactorSet(_factor.numerator, _factor.denominator);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 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.