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

Incorrect Token Allocation Tracking in Auction Removal Process in `SpiceAuction::removeAuctionConfig()` Leads to Locked Funds and DoS of Future Auctions

nisedo

Summary

The SpiceAuction contract fails to update the _totalAuctionTokenAllocation when removing an auction during its cooldown period, leading to an inflated and inaccurate token allocation record.

Vulnerability Details

In the startAuction function, the contract updates the total auction token allocation:

/**
* @notice Start auction. Checks caller is set config starter. Address zero for anyone to call
*/
function startAuction() external override {
uint256 epochId = _currentEpochId;
/// @dev config is always set for next auction
/// @notice Configuration is set before auctions so configId = currentEpochId + 1;
SpiceAuctionConfig storage config = auctionConfigs[epochId+1];
if (config.duration == 0) { revert CannotStartAuction(); }
/// @notice only starter
if (config.starter != address(0) && msg.sender != config.starter) { revert CommonEventsAndErrors.InvalidAccess(); }
/// @notice enough wait period since last auction
if (epochId > 0) {
/// @dev `_currentEpochId` is still last epoch
EpochInfo memory lastEpochInfo = epochs[epochId];
/// use waitperiod from last auction config
uint64 _waitPeriod = auctionConfigs[epochId].waitPeriod;
if (lastEpochInfo.endTime + _waitPeriod > block.timestamp) { revert CannotStartAuction(); }
} else {
/// For first auction
if (_deployTimestamp + config.waitPeriod > block.timestamp) { revert CannotStartAuction(); }
}
(,address auctionToken) = _getBidAndAuctionTokens(config);
uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[auctionToken];
uint256 balance = IERC20(auctionToken).balanceOf(address(this));
uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]);
if (config.activationMode == ActivationMode.AUCTION_TOKEN_BALANCE) {
if (config.minimumDistributedAuctionToken == 0) { revert MissingAuctionTokenConfig(); }
}
if (epochAuctionTokenAmount < config.minimumDistributedAuctionToken) { revert NotEnoughAuctionTokens(); }
// epoch start settings
// now update currentEpochId
epochId = _currentEpochId = _currentEpochId + 1;
EpochInfo storage info = epochs[epochId];
uint128 startTime = info.startTime = uint128(block.timestamp) + config.startCooldown;
uint128 endTime = info.endTime = startTime + config.duration;
info.totalAuctionTokenAmount = epochAuctionTokenAmount;
// Keep track of total allocation auction tokens per epoch
❌ _totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + epochAuctionTokenAmount; //@audit not updated in removeAuctionConfig()
​
emit AuctionStarted(epochId, msg.sender, startTime, endTime, epochAuctionTokenAmount);
}

However, the removeAuctionConfig function, which can be called during an auction's cooldown period, does not reverse this allocation:

/// @notice Remove auction config set for last epoch
function removeAuctionConfig() external override onlyDAOExecutor {
/// only delete latest epoch if auction is not started
uint256 id = _currentEpochId;
EpochInfo storage info = epochs[id];
// _currentEpochId = 0
if (info.startTime == 0) { revert InvalidConfigOperation(); }
// Cannot reset an ongoing auction
if (info.isActive()) { revert InvalidConfigOperation(); }
/// @dev could be that `auctionStart` is triggered but there's cooldown, which is not reached (so can delete epochInfo for _currentEpochId)
// or `auctionStart` is not triggered but `auctionConfig` is set (where _currentEpochId is not updated yet)
bool configSetButAuctionStartNotCalled = auctionConfigs[id+1].duration > 0;
if (!configSetButAuctionStartNotCalled) {
/// @dev unlikely because this is a DAO execution, but avoid deleting old ended auctions
if (info.hasEnded()) { revert AuctionEnded(); }
/// auction was started but cooldown has not passed yet
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, id);
} else {
// `auctionStart` is not triggered but `auctionConfig` is set
id += 1;
delete auctionConfigs[id];
emit AuctionConfigRemoved(id, 0);
}
}

This oversight results in the _totalAuctionTokenAllocation remaining inflated with the allocation of the removed auction.

Importantly, the existing recovery functions recoverToken and recoverAuctionTokenForZeroBidAuction cannot be used to recover these tokens:

  1. recoverToken calculates the recoverable amount as:

uint256 maxRecoverAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[token]);

The inflated totalAuctionTokenAllocation prevents recovery of the locked tokens.

  1. recoverAuctionTokenForZeroBidAuction only works for ended auctions with zero bids, not for cancelled auctions in the cooldown period.

This leaves no mechanism within the contract to correct the inflated _totalAuctionTokenAllocation, exacerbating the issue over time.

Scenario

  • Admin sets up Auction 1 with 1000 TGLD tokens

  • startAuction is called, setting _totalAuctionTokenAllocation[TGLD] = 1000

  • The auction enters cooldown period

  • Admin decides to cancel Auction 1 and calls removeAuctionConfig

  • _totalAuctionTokenAllocation[TGLD] remains at 1000, despite the auction being cancelled

  • Admin sets up Auction 2

  • startAuction for Auction 2 fails, as the contract believes all 1000 TGLD are still allocated to the non-existent Auction 1

  • Previous 1000 TGLD are locked in the contract

Impact

  1. Token Lockup: Future auctions will have access to fewer tokens than actually available, as the system believes more tokens are allocated than reality.

uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]);

This calculation in startAuction will result in a lower epochAuctionTokenAmount than it should be.

  1. DoS of next auctions: If the next auction is started with the same balance as the removed one, epochAuctionTokenAmount will be 0, which would deny the starting of new auction because of this check:

//@audit minimumDistributedAuctionToken can't be 0 in setAuctionConfig()
if (epochAuctionTokenAmount < config.minimumDistributedAuctionToken) { revert NotEnoughAuctionTokens(); }

Recommendations

Implement a proper deallocation in the removeAuctionConfig function:

if (!configSetButAuctionStartNotCalled) {
if (info.hasEnded()) { revert AuctionEnded(); }
SpiceAuctionConfig storage config = auctionConfigs[id];
(, address auctionToken) = _getBidAndAuctionTokens(config);
_totalAuctionTokenAllocation[auctionToken] -= info.totalAuctionTokenAmount;
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, id);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

missing totalAuctionTokenAllocation deduction in removeAuctionConfig leads to stuck funds

Support

FAQs

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