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

removeAuctionConfig Fails to Deduct _totalAuctionTokenAllocation, Resulting in Locked Funds

0xCiphky

Vulnerability Details:

The removeAuctionConfig function in the SpiceAuction contract allows the DAO executor to remove an auction configuration. This can occur in two scenarios:

  • When auctionStart is not triggered, and auctionConfig is set.

  • When auctionStart is triggered, and auctionConfig is set.

However, the second scenario is not handled correctly. Currently, the auctionConfigs[id] and epochs[id] are deleted, and the _currentEpochId is decremented by one. However, it fails to deduct totalAuctionTokenAmount from the _totalAuctionTokenAllocation[auctionToken], which tracks the reserved amount of tokens in the contract for claims from previous auctions.

Consequently, the _totalAuctionTokenAllocation for the canceled auction will still be reserved, but since the auction was canceled, these funds will be stuck in the contract forever.

function removeAuctionConfig() external override onlyDAOExecutor {
...
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 {
...
}
}

Impact:

The reserved tokens for the canceled auction remain locked in the contract, making them inaccessible for future claims. This results in a permanent loss of these funds

Proof Of Concept

function test_removeConfig() public {
// Check initial _totalAuctionTokenAllocation is 0
assertEq(spice._totalAuctionTokenAllocation(address(templeGold)), 0);
// Start auction
ISpiceAuction.SpiceAuctionConfig memory _config = _startAuction(true, true);
// Check _totalAuctionTokenAllocation is 100, funds sent to contract for auction
assertEq(spice._totalAuctionTokenAllocation(address(templeGold)), 100 ether);
// Get epoch info
uint256 currentEpoch = spice.currentEpoch();
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(currentEpoch);
// warp below start time (this includes cooldown)
vm.warp(info.startTime - 10 seconds);
vm.startPrank(daoExecutor);
// Remove auction config
spice.removeAuctionConfig();
// Check _totalAuctionTokenAllocation is 100 after removing config, should be 0
assertEq(spice._totalAuctionTokenAllocation(address(templeGold)), 100 ether);
}

Tools Used:

  • Manual analysis

  • Foundry

Recommendation:

To resolve this issue, ensure that the removeAuctionConfig function correctly updates the _totalAuctionTokenAllocation when an auction is canceled. Specifically, deduct the info.totalAuctionTokenAmount from the _totalAuctionTokenAllocation[auctionToken] to release the reserved tokens.

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
(, address auctionToken) = _getBidAndAuctionTokens(auctionConfigs[id]); // add here
_totalAuctionTokenAllocation[auctionToken] -= info.totalAuctionTokenAmount; // add here
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);
}
}
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

Appeal created

0xCiphky Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
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.