TempleGold

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

[L-4] Incorrect event emission SpiceAuction::removeAuctionConfig() event AuctionConfigRemoved

github link

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L131

Vulnerability Details

The removeAuctionConfig() function in the SpiceAuction contract is responsible for removing the auction configuration. However, the function emits an AuctionConfigRemoved event with incorrect parameters. This can lead to confusion and errors when tracking the removal of auction configurations, as the event does not accurately reflect the state changes.

Impact:

Incorrect event emission has several potential impacts like

  • Tracking and Debugging: It can make it difficult to track and debug the removal of auction configurations, as the emitted event does not provide accurate information.

  • User Confusion: Users and developers relying on event logs for accurate and up-to-date information may face confusion and errors, affecting their interaction with the contract.

Proof of Concept:

Below is the removeAuctionConfig() function, and in L131, which i also showed with @> you can see that the event was emitted wrongly

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);
}
}

Recommended Mitigation:

To address this issue, it is recommended to ensure that the AuctionConfigRemoved event is emitted with accurate parameters that reflect the state changes. This can be achieved by reviewing the logic and parameters passed to the event emission.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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