TempleGold

​

The `setVestingFactor` function can be called without invoking the `mint` function, which may result in the minting of an incorrect number of TempleGold tokens. If the `vestingFactor` is increased, the new vesting would apply to a period in the past, potentially minting extra tokens. Conversely, reducing the `vestingFactor` during an ongoing vesting process can lead to fewer tokens being minted, undermining the fairness for the valid period between `lastMintTimestamp`.

​

​

The root cause of the vulnerability is that the `setVestingFactor` function does not require the `mint` function to be called beforehand to update the `lastMintTimestamp`.

​

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

​

// File: protocol/contracts/templegold/TempleGold.sol

130: function setVestingFactor(VestingFactor calldata _factor) external override onlyOwner {

131: if (_factor.numerator == 0 || _factor.denominator == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }

132: if (_factor.numerator > _factor.denominator) { revert CommonEventsAndErrors.InvalidParam(); }

133: vestingFactor = _factor; // <= FOUND: `setVestingFactor` without calling mint might mint more TempleGold tokens after

134: /// @dev initialize

135: if (lastMintTimestamp == 0) { lastMintTimestamp = uint32(block.timestamp); } // <= FOUND: lastMintTimestamp not always up-to-date

136: emit VestingFactorSet(_factor.numerator, _factor.denominator);

137: }

​

Let us walk through the issue with the following scenarios:

​

1. Alice (admin) calls `setVestingFactor` to increase the `vestingFactor`.

2. Since `mint` was not called before, the `lastMintTimestamp` remains outdated.

3. The increased `vestingFactor` now applies retroactively from the `lastMintTimestamp`, potentially allowing minting of extra tokens.

​

Similarly:

​

1. Alice calls `setVestingFactor` to decrease the `vestingFactor`.

2. If `mint` was not called before, the `lastMintTimestamp` remains outdated.

3. The decreased `vestingFactor` now applies retroactively from the `lastMintTimestamp`, leading to fewer tokens being minted for the valid period.

​

​

This vulnerability can result in either the minting of an excessive number of tokens or fewer tokens than intended. Both scenarios can lead to financial discrepancies and unfair token distributions.

​

​

Manual Review

​

​

To mitigate this issue, enforce the `mint` function to update the `lastMintTimestamp` to the latest block before calling `setVestingFactor`. This ensures that any changes to the `vestingFactor` are applied correctly from the current block timestamp, preventing retroactive token minting discrepancies.