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

Incorrect templeGold minting due to unresolved accumulation in `TempleGold::setVestingFactor`

kildren

Relevant GitHub Links

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

Summary

templeGold is minted at a rate strictly following the VestingFactor. However, when the operators call TempleGold::setVestingFactor to modify the VestingFactor, the tokens accumulated based on the previous factor are not resolved. This miscommunication results in an incorrect calculation of mint volumes, as the accumulated time is multiplied by the new VestingFactor, making the emission of templeGold unpredictable.

Vulnerability Details

At Line 130, the VestingFactor is updated directly without resolving the tokens to be minted based on the accumulated time and the previous VestingFactor. This causes the accumulated time to be incorrectly calculated with the new VestingFactor, leading to discrepancies between expected and actual minting amounts.

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);
}

POC

We will compare the actual minting quantity of the protocol with the expected minting quantity:

  1. On the first day, minting follows the normal vesting factor rate.

  2. On the second day, the vesting factor is doubled.

  3. Expected behavior is that the first day follows the old rate and the second day follows the doubled rate, making the total equivalent to three days at the normal rate.

  4. However, the protocol will incorrectly assume both days are minted at the doubled rate.

Place the following test into TempleGold.t.sol::TempleGoldTest:

function test_setVestingFactorUnresolved_tgld() public {
vm.startPrank(executor);
ITempleGold.VestingFactor memory _factor = _getVestingFactor();
// The _getVestingFactor() sets the rate too fast, distributing everything within ten time units.
_factor.numerator = 1 ether;
_factor.denominator = 100_000_000_000 ether;
​
templeGold.setVestingFactor(_factor);
// pass one day
uint256 mintTime = block.timestamp + 1 days;
vm.warp(mintTime);
uint256 one_day_mint_amount = templeGold.getMintAmount();
emit log_named_uint("mintAmount: ", templeGold.getMintAmount());
​
_factor.numerator *= 2;
templeGold.setVestingFactor(_factor);
// pass one day again
mintTime += 1 days;
vm.warp(mintTime);
uint256 final_mint_amount = templeGold.getMintAmount();
​
// expected:
// first day: 1 day * _factor
// second day: 1 day * 2 * _factor
// final : first day + second day = 3 * one_day_mint_amount
assertGt(final_mint_amount, 3 * one_day_mint_amount);
// actual:
// final : 2 day * 2 * _factor = 4 * one_day_mint_amount
assertEq(final_mint_amount, 4 * one_day_mint_amount);
}

Impact

This issue can lead to significant fluctuations in the issuance rate of templeGold, causing unpredictable minting patterns that can destabilize the protocol and undermine user confidence.

Tools Used

Manual Review.

Recommendations

Resolve Pending Mints Before Updating VestingFactor: Call mint() to resolve any tokens accumulated under the previous vesting factor before updating to a new VestingFactor. This ensures that the calculation for the new vesting period starts accurately.

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

(Optional) Restrict VestingFactor Changes: Implement constraints on how significantly the VestingFactor can be changed relative to the previous factor and impose time-based restrictions on how frequently it can be updated. These measures will enhance the predictability and stability of templeGold emission.

By resolving any accumulated tokens before setting the new VestingFactor and by restricting the frequency and extent of VestingFactor changes, the protocol can maintain a stable and predictable minting process, thereby preserving the stability and user trust of the templeGold system.

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.