TempleGold

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

`_totalAuctionTokenAllocation` is not updated in case of auction config removal in `SpicyAuction` contract

Summary

The spicy auction contract has offered some ways to recover tokens in case of emergency. The reccoverTokens function allows the exector role to withdraw tokens from the contract. In case of the token is auction tokens, and the startAuction function has been called, but during the cooldown period, according to the comments by devs, removeAuctionConfig shall be called as a solution to cancel an auction. However, when auctions are cancelled, important state variables such as _totalAuctionTokenAllocation is not updated, and will cause further calculation errors.

Vulnerability Details

With the assumption of that the auction has been started, but on cooldown period, we can first look at the startAuction function in SpicyAuction contract:

(,address auctionToken) = _getBidAndAuctionTokens(config);
// @note can this exceeds current balance?
uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[auctionToken];
uint256 balance = IERC20(auctionToken).balanceOf(address(this));
// @note amount available for auction = current balance - (reserved amounts --> allocated amount - claimed)
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;

As we can see, the total available tokens in the round of auction is calculated by the subtracting the "reserved amounts" from the current total balance. After passing various checks, the epochAuctionTokenAmount is added to the global variable _totalAuctionTokenAllocation[auctionToken].

However, continue with the assumption, and some emergency case happens, the DAO decides to cancel the auction. Since the auction has been started, it would be the best to call removeAuctionConfig function, where we can see, simply removes the entire struct value in the map:

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 causes an issue, since _totalAuctionTokenAllocation[auctionToken] is updated when auction is started, when the same auction is removed, and without even really starting, the total allocated tokens should also be deducted. As this value is also used to determine the auctioned amount for next round, this can cause bidders to get less than they actually can, and also a DoS in some cases.

For example, the math for updating allocated tokens is:

_totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + epochAuctionTokenAmount;

which is essentially:

_totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]) = balance + _claimedAuctionTokens[auctionToken]

This makes the allocated amount for potentially next round, to be larger than the current balance. When a started auction is cancelled, or has its config removed, the DAO decides to correctly set a new config and start it right away, but they would find their call to startAuction always revert, as there will be not enough balance in the contract to cover the auction for this new config, despite the previous auction is cancelled, and the allocated amount should be subtracted.

Impact

This will cause bidders to get less auctioned tokens, and can also cause DoS if the DAO decides to start a new auction after removing a previous one.

Tools Used

Manual review.

Recommendations

When auction config is removed, also deduct the correspond allocation amount.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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.