Summary:
The SpiceAuction contract's setAuctionConfig function fails to update the epochs mapping with EpochInfo, causing the removeAuctionConfig function to always revert. This creates a Denial of Service (DoS) vulnerability that prevents the removal of auction configurations.
Vulnerability Details:
The setAuctionConfig function does not store EpochInfo in the epochs mapping, leading to a mismatch in expected state. Consequently, the removeAuctionConfig function cannot find the necessary EpochInfo, causing it to revert due to the first if condition always failing.
Code Snippet:
function setAuctionConfig(SpiceAuctionConfig calldata _config) external onlyDAOExecutor {
uint256 currentEpochIdCache = _currentEpochId;
if (currentEpochIdCache > 0) {
EpochInfo storage info = epochs[currentEpochIdCache];
if (info.isActive()) revert InvalidConfigOperation();
}
if (
_config.duration < MINIMUM_AUCTION_PERIOD || _config.duration > MAXIMUM_AUCTION_DURATION
|| _config.waitPeriod > MAXIMUM_AUCTION_WAIT_PERIOD
) revert CommonEventsAndErrors.InvalidParam();
if (_config.waitPeriod == 0 || _config.minimumDistributedAuctionToken == 0) {
revert CommonEventsAndErrors.ExpectedNonZero();
}
if (_config.recipient == address(0)) revert CommonEventsAndErrors.InvalidAddress();
currentEpochIdCache += 1;
@>
auctionConfigs[currentEpochIdCache] = _config;
emit AuctionConfigSet(currentEpochIdCache, _config);
}
function removeAuctionConfig() external override onlyDAOExecutor {
uint256 id = _currentEpochId;
@> EpochInfo storage info = epochs[id];
if (info.startTime == 0) revert InvalidConfigOperation();
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, id);
} else {
id += 1;
delete auctionConfigs[id];
emit AuctionConfigRemoved(id, 0);
}
}
Impact:
This vulnerability can significantly disrupt the normal operations of the SpiceAuction contract. By preventing the removal of outdated or unwanted auction configurations. Users and stakeholders are forced to participate in that auction configuration, and the contract administrators cannot update auction parameters. This breakdown in functionality undermines the trust and reliability of the auction platform, potentially causing financial loss and reputational damage to the managing DAO and the broader ecosystem.
Proof Of Concept:
DAO administrators call the setAuctionConfig function to set a new auction configuration.
The setAuctionConfig function increments the currentEpochIdCache and stores the configuration in the auctionConfigs mapping but fails to update the epochs mapping with EpochInfo.
The removeAuctionConfig function is called to remove the auction configuration.
The removeAuctionConfig function attempts to retrieve the EpochInfo from the epochs mapping using the _currentEpochId but fails to find the necessary information, causing the function to revert.
The removeAuctionConfig function reverts, preventing the removal of the auction configuration and causing a Denial of Service (DoS) vulnerability.
Proof Of Code:
Place the following code in the SpiceAuction.t.sol contract:
To run the test, follow the commands below:
forge test --mt test_FortisAudits_removeAuctionConfig_Reverts -vvvv
function test_FortisAudits_removeAuctionConfig_Reverts() public {
ISpiceAuction.SpiceAuctionConfig memory config = _getAuctionConfig();
vm.startPrank(daoExecutor);
spice.setAuctionConfig(config);
vm.stopPrank();
uint256 id = spice.currentEpoch();
spice.getAuctionConfig(id);
assertEq(id, 0);
spice.getEpochInfo(id);
vm.startPrank(daoExecutor);
vm.expectRevert(abi.encodeWithSelector(ISpiceAuction.InvalidConfigOperation.selector));
spice.removeAuctionConfig();
vm.stopPrank();
vm.startPrank(alice);
vm.warp(block.timestamp + config.waitPeriod);
IERC20 auctionToken = IERC20(_getAuctionToken(config.isTempleGoldAuctionToken, daiToken));
dealAdditional(auctionToken, address(spice), 100 ether);
spice.startAuction();
vm.warp(block.timestamp + config.duration + config.startCooldown + 1);
vm.stopPrank();
vm.startPrank(daoExecutor);
vm.expectRevert(abi.encodeWithSelector(IAuctionBase.AuctionEnded.selector));
spice.removeAuctionConfig();
vm.stopPrank();
}
Tools Used:
Manual code review
Foundry
Recommendation:
To mitigate this issue, the setAuctionConfig function should be updated to store EpochInfo in the epochs mapping. This update will allow the removeAuctionConfig function to retrieve the necessary information and remove auction configurations as intended.
Here is the recommended mitigation:
function setAuctionConfig(SpiceAuctionConfig calldata _config) external onlyDAOExecutor {
uint256 currentEpochIdCache = _currentEpochId;
if (currentEpochIdCache > 0) {
EpochInfo storage info = epochs[currentEpochIdCache];
if (info.isActive()) revert InvalidConfigOperation();
}
if (
_config.duration < MINIMUM_AUCTION_PERIOD || _config.duration > MAXIMUM_AUCTION_DURATION
|| _config.waitPeriod > MAXIMUM_AUCTION_WAIT_PERIOD
) revert CommonEventsAndErrors.InvalidParam();
if (_config.waitPeriod == 0 || _config.minimumDistributedAuctionToken == 0) {
revert CommonEventsAndErrors.ExpectedNonZero();
}
if (_config.recipient == address(0)) revert CommonEventsAndErrors.InvalidAddress();
currentEpochIdCache += 1;
+ EpochInfo storage info = epochs[currentEpochIdCache]; // Add this line
+ uint128 startTime = info.startTime = uint128(block.timestamp) + _config.startCooldown;
+ info.endTime = startTime + _config.duration;
+ info.totalAuctionTokenAmount = _config.minimumDistributedAuctionToken;
+ auctionConfigs[currentEpochIdCache] = _config;
emit AuctionConfigSet(currentEpochIdCache, _config);
}
function removeAuctionConfig() external override onlyDAOExecutor {
/// only delete latest epoch if auction is not started
uint256 id = _currentEpochId;
- EpochInfo storage info = epochs[id];
+ EpochInfo storage info = epochs[id + 1];
.
.
.
.
}
