TempleGold

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

## [M-1] SpiceAuction::removeAuctionConfig reverts for First ever Auction

summary

The first-ever created Auction Config in SpiceAuction.sol will revert if an attempt is made to remove it via SpiceAuction::removeAuctionConfig.

Description:

  • In the SpiceAuction::removeAuctionConfig function, there is a condition that causes the function to revert the condition is if info.startTime == 0 for the current epoch. The current epoch is derived from the state variable AuctionBase::_currentEpochId, which, according to the NatSpec comment, represents the last epoch:

  • However, for the first-ever created auction configuration, there are no preceding epochs. Consequently, the daoExecutor is unable to remove the initial auction configuration since EpochInfo.startTime is only updated within the SpiceAuction::startAuction function.

Impact:
Should the daoExecutor make an error in the initial auction configuration, the contract becomes effectively unusable, as the configuration cannot be removed or corrected. This inability to amend or remove the first auction configuration without redeploying the contract severely impairs its functionality and flexibility.

Proof of Concept:
Below is the removeAuctionConfig() function highlighting the problematic check:

function removeAuctionConfig() external override onlyDAOExecutor {
/// only delete latest epoch if auction is not started
uint256 id = _currentEpochId;
EpochInfo storage info = epochs[id];
@> if (info.startTime == 0) revert InvalidConfigOperation();
// Cannot reset an ongoing auction
if (info.isActive()) revert InvalidConfigOperation();
bool configSetButAuctionStartNotCalled = auctionConfigs[id + 1].duration > 0;
if (!configSetButAuctionStartNotCalled) {
if (info.hasEnded()) revert AuctionEnded();
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, _currentEpochId);
} else {
id += 1;
delete auctionConfigs[id];
emit AuctionConfigRemoved(id, _currentEpochId);
}
}

this below POC shows first auction ever reverting on calling removeAuctionConfig

function test_firstConfigReverts() public {
// setting config
vm.startPrank(daoExecutor);
ISpiceAuction.SpiceAuctionConfig memory _config = _getAuctionConfig();
spice.setAuctionConfig(_config);
// reverts when removing first ever auction config
vm.expectRevert();
spice.removeAuctionConfig();
vm.startPrank(alice);
vm.warp(block.timestamp + _config.waitPeriod);
IERC20 auctionToken = IERC20(_getAuctionToken(_config.isTempleGoldAuctionToken, daiToken));
dealAdditional(auctionToken, address(spice), 100 ether);
uint256 epoch = spice.currentEpoch();
IAuctionBase.EpochInfo memory epochInfo = spice.getEpochInfo(epoch);
spice.startAuction();
epochInfo = spice.getEpochInfo(epoch + 1);
vm.warp(epochInfo.endTime);
_config = _getAuctionConfig();
_config.starter = address(0);
_config.startCooldown = 0;
vm.startPrank(daoExecutor);
spice.setAuctionConfig(_config);
// able to remove now
spice.removeAuctionConfig();
}
  • Include the above code in your test file, and you will observe that it passes, indicating that the expectRevert condition was correctly anticipated.

Recommended Mitigation:
To address this issue, it is recommended to update the function logic to allow the removal of the first auction config.

Updates

Lead Judging Commences

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

`removeAuctionConfig` can't remove the first added `SpiceAuctionConfig` which in the end leads to inability to recover the funds associated to that auction

Support

FAQs

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