TempleGold

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

Potentially Inconvenient auction durations due to possible low wait period in SpiceAuction

Summary

The SpiceAuction contract allows setting a very low wait period in setAuctionConfigfor starting new auctions. Although the wait period cannot be zero, it doesn't make much difference if it is 1 or 2, setting it too low can lead to excessively short auction durations, potentially making the auction process inconvenient for users.

Vulnerability Details

In the SpiceAuction contract, the setAuctionConfig function ensures that the waitPeriod is non-zero. However, there is no enforced minimum value beyond being non-zero, meaning it can be set to a very low value. This lack of constraint can result in auctions that are too short, making it difficult/impossible for participants to bid.

Relevant code snippet:

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

function setAuctionConfig(SpiceAuctionConfig calldata _config) external onlyDAOExecutor {
/// @dev epoch Id is only updated when auction starts.
/// @dev cannot set config for past or ongoing auction
uint256 currentEpochIdCache = _currentEpochId;
if (currentEpochIdCache > 0) {
EpochInfo storage info = epochs[currentEpochIdCache];
/// Cannot set config for ongoing auction
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(); }
/// @dev startCooldown can be zero
> if (_config.waitPeriod == 0 //@auditSUBMITTED LOW/MID no much difference if waitPeriod is 1 or 0, so lower bound REQUIRED
|| _config.minimumDistributedAuctionToken == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (_config.recipient == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
currentEpochIdCache += 1;
auctionConfigs[currentEpochIdCache] = _config;
emit AuctionConfigSet(currentEpochIdCache, _config);
}

Impact

If the wait period is set too low, auctions may end too quickly for participants to place their bids, reducing the effectiveness and fairness of the auction process. While this is primarily a configuration issue that depends on the actions of the daoexecutor, it can still disrupt the auction flow and user experience.

Likelihood looks LOW/MEDIUM because daoexecutorhas to set it to low value and then function startAuctionhas to be called (but if starter is not set it can be called by anyone).

The impact looks like a LOW because no funds are lost and a new auction can always be run.

Tools Used

Manual Review

Recommendations

Add a sensible lower bound check for the _config.waitPeriod in the setAuctionConfig function to prevent it from being set too low.

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.