TempleGold

TempleDAO
Foundry
25,000 USDC
View results
Submission Details
Severity: high
Invalid

Due to lack of validation, a malicious bidder can intentionally increasing the `info.totalAuctionTokenAmount` and therefore the malicious bidder can manipulate the `claimAmount` to be received

Summary

Due to lack of validation, a malicious bidder can intentionally increasing the info.totalAuctionTokenAmount and therefore the malicious bidder can manipulate the claimAmount to be received

Vulnerability Details

Within the TempleGold contract, the MAX_SUPPLY would be defined as 1_000_000_000 ether (1 billion) like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L47

/// @notice 1B max supply
uint256 public constant MAX_SUPPLY = 1_000_000_000 ether; // 1B

Within the DaiGoldAuction contract, the nextAuctionGoldAmount would be defined to keep track of next epoch auction Temple Gold amount like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L41

/// @notice Keep track of next epoch auction Temple Gold amount
uint256 public override nextAuctionGoldAmount;

Within the DaiGoldAuction#distributeGold(), the DaiGoldAuction#_distributeGold() would be called like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L300

/**
* @notice Mint and distribute TGOLD
*/
function distributeGold() external {
_distributeGold(); ///<------------- @audit
}

Within the DaiGoldAuction#_distributeGold(), the TempleGold#mint() would be called like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L305

function _distributeGold() private {
/// @dev no op silent fail if nothing to distribute
templeGold.mint(); ///<------------- @audit
}

Within the TempleGold#mint(), the TempleGold#_distribute() would be called like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L155

function mint() external override onlyArbitrum {
...
DistributionParams storage distributionParamsCache = distributionParams;
...
uint256 mintAmount = _getMintAmount(vestingFactorCache);
/// @dev no op silently
if (!_canDistribute(mintAmount)) { return; }
...
_distribute(distributionParamsCache, mintAmount); ///<------------- @audit
}

Within the TempleGold#_distribute(), the TempleGoldStaking#notifyDistribution() would be called with the stakingAmount and the DaiGoldAuction#notifyDistribution() would be called with the escrowAmount like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L230
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L236

function _distribute(DistributionParams storage params, uint256 mintAmount) private {
uint256 stakingAmount = TempleMath.mulDivRound(params.staking, mintAmount, DISTRIBUTION_DIVISOR, false);
if (stakingAmount > 0) {
_mint(address(staking), stakingAmount);
staking.notifyDistribution(stakingAmount); ///<------------ @audit - NOTE: "staking" is the "TempleGoldStaking" contract
}
uint256 escrowAmount = TempleMath.mulDivRound(params.escrow, mintAmount, DISTRIBUTION_DIVISOR, false);
if (escrowAmount > 0) {
_mint(address(escrow), escrowAmount);
escrow.notifyDistribution(escrowAmount); ///<------------- @audit - NOTE: "escrow" is the "DaiGoldAuction" contract.
}
...
_totalDistributed += mintAmount;

Within the DaiGoldAuction#notifyDistribution(), the nextAuctionGoldAmount would be incremented by adding a given amount (which is the escrowAmount assigned in the TempleGold#_distribute() above) like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L174

function notifyDistribution(uint256 amount) external override {
if (msg.sender != address(templeGold)) { revert CommonEventsAndErrors.InvalidAccess(); }
/// @notice Temple Gold contract mints TGLD amount to contract before calling `notifyDistribution`
nextAuctionGoldAmount += amount; ///<------------- @audit

Within the DaiGoldAuction#startAuction(), the nextAuctionGoldAmount would be stored into the totalGoldAmount and then it would be stored into the info.totalAuctionTokenAmount (This means that the nextAuctionGoldAmount would indirectly be stored into the info.totalAuctionTokenAmount) like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L114
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L121

function startAuction() external override {
...
_distributeGold();
uint256 totalGoldAmount = nextAuctionGoldAmount; ///<------------- @audit
nextAuctionGoldAmount = 0;
...
EpochInfo storage info = epochs[epochId];
info.totalAuctionTokenAmount = totalGoldAmount; ///<------------- @audit

After the bidding period would be finished, a user would call the DaiGoldAuction#claim() to claim the share of TempleGold for epoch.
Within the DaiGoldAuction#claim(), the claimAmount would be calculated based on the formula, which includes info.totalAuctionTokenAmount, and then it would be transferred to the user (msg.sender) like this:
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/DaiGoldAuction.sol#L161-L162

function claim(uint256 epochId) external virtual override {
/// @notice cannot claim for current live epoch
EpochInfo storage info = epochs[epochId];
...
uint256 claimAmount = bidTokenAmount.mulDivRound(info.totalAuctionTokenAmount, info.totalBidTokenAmount, false); ///<------------- @audit
templeGold.safeTransfer(msg.sender, claimAmount); ////<------------- @audit - TGLD is transferred to the bidder (msg.sender)

The DaiGoldAuction#distributeGold() above is supposed to be called by only privileged-user.
However, within the DaiGoldAuction#distributeGold(), there is no validation heck whether or not the caller (msg.sender) of the DaiGoldAuction#distribute() is a privileged-user.

This allow anyone to call the DaiGoldAuction#distributeGold() to mint TGLD to the both contracts (TempleGoldStaking and the DaiGoldAuction) and increase the nextAuctionGoldAmount as much as the user want.

If the malicious bidder would repeadedly call the DaiGoldAuction#distributeGold() before the auction get started, the amount of TGLD-minted and the nextAuctionGoldAmount can be increased to the amount, which is close to the max supply of the TGLD is 1 billion (MAX_SUPPLY = 1_000_000_000 ether),

After the malicious action above, if an auctionStarter would call the DaiGoldAuction#startAuction(), the nextAuctionGoldAmount, which is close to the max supply of the TGLD is 1 billion (MAX_SUPPLY = 1_000_000_000 ether), would be stored into the totalGoldAmount and then it would be stored into the info.totalAuctionTokenAmount in the DaiGoldAuction#startAuction().

Since the claimAmount would be calculated in the DaiGoldAuction#claim() based on the formula-including the info.totalAuctionTokenAmount, the claimAmount can be maximized due to that the info.totalAuctionTokenAmount has been the amount, which is close to the max supply of the TGLD is 1 billion (MAX_SUPPLY = 1_000_000_000 ether):

uint256 claimAmount = bidTokenAmount.mulDivRound(info.totalAuctionTokenAmount, info.totalBidTokenAmount, false);

Hence, after the bidding period is finished, the malicious bidder can be successful to (claim and) receive so much amount of TGLD by intentionally increasing the info.totalAuctionTokenAmount and therefore the malicious bidder can manipulate the claimAmount to be received.

Attack scenario

Let's say Alice is an auctionStarter and Bob is a malicious bidder who would like to maximize the amount of TGLD to be claimed.

  • 1/ Now, the auction is not started.

  • 2/ Bob would repeadedly call the DaiGoldAuction#distributeGold().

    • By repeadedly doing so, eventually, the nextAuctionGoldAmount would be increased to 999_000_000 ether, which is close to the max supply of the TGLD (MAX_SUPPLY = 1_000_000_000 ether).

  • 3/ Alice would call the DaiGoldAuction#startAuction() to start an auction.

    • At this point, the increased nextAuctionGoldAmount (999_000_000 ether) would be stored into the totalGoldAmount and then it would be stored into the info.totalAuctionTokenAmount.

  • 4/ Bob (and other bidders) would bid.

  • 5/ The bidding period is finished.

  • 6/ Bob would call the DaiGoldAuction#claim()

    • The claimAmount is calculated based on the formula, which includes the info.totalAuctionTokenAmount.

    • Since the increased nextAuctionGoldAmount (999_000_000 ether) has been stored into the info.totalAuctionTokenAmount when the step 3/, the claimAmount can also be increased (maximized) like this:

uint256 claimAmount = bidTokenAmount.mulDivRound(info.totalAuctionTokenAmount, info.totalBidTokenAmount, false);
uint256 claimAmount = bidTokenAmount.mulDivRound(999_000_000 ether, info.totalBidTokenAmount, false);
  • 7/ As a result, Bob could receive so much amount of TGLD.

(NOTE: By the way, in this case, other bidders could alsov receive so much amount of TGLD)

Impact

A malicious bidder can intentionally increasing the info.totalAuctionTokenAmount and therefore the malicious bidder can manipulate the claimAmount to be received.

Tools Used

  • Foundry

Recommendations

Within the DaiGoldAuction#distributeGold(), consider adding an access control modifier (i.e. onlyElevatedAccess) - so that only privileged-user can call the DaiGoldAuction#distributeGold() like this:

+ function distributeGold() onlyElevatedAccess {
- function distributeGold() external {
_distributeGold();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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