Summary
If no bids are placed on DaiGoldAuction in an entire epoch, the auctioned tokens will get stuck in the contact, with no way to recover them.
Vulnerability Details
The startAuction() function is in charge of starting a new auction, and allocating the auctioned tokens. When called, a new auction starts, and the field totalAuctionTokenAmount of TempleGOLD tokens is set for the new auction. Once the auction finishes, the totalAuctionTokenAmount can be claimed by the bidders, proportionally to their bids.
However, if there are no bidders, nobody can claim the allocated auction tokens because any wallet calling claim() will get a CommonEventsAndErrors.ExpectedNonZero() revert message.
* @notice Claim share of Temple Gold for epoch
* Can only claim for past epochs, not current auction epoch.
* @param epochId Id of epoch
*/
function claim(uint256 epochId) external virtual override {
EpochInfo storage info = epochs[epochId];
if (!info.hasEnded()) { revert CannotClaim(epochId); }
if (info.startTime == 0) { revert InvalidEpoch(); }
uint256 bidTokenAmount = depositors[msg.sender][epochId];
if (bidTokenAmount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
delete depositors[msg.sender][epochId];
uint256 claimAmount = bidTokenAmount.mulDivRound(info.totalAuctionTokenAmount, info.totalBidTokenAmount, false);
templeGold.safeTransfer(msg.sender, claimAmount);
emit Claim(msg.sender, epochId, bidTokenAmount, claimAmount);
}
Moreover, the startAuction() function does not carry over the unallocated totalAuctionTokenAmount to the next round if no bids are placed, which will make these auction tokens stuck in the DaiGoldAuction contract.
function startAuction() external override {
if (auctionStarter != address(0) && msg.sender != auctionStarter) { revert CommonEventsAndErrors.InvalidAccess(); }
EpochInfo storage prevAuctionInfo = epochs[_currentEpochId];
if (!prevAuctionInfo.hasEnded()) { revert CannotStartAuction(); }
AuctionConfig storage config = auctionConfig;
if (_currentEpochId > 0 && (prevAuctionInfo.endTime + config.auctionsTimeDiff > block.timestamp)) {
revert CannotStartAuction();
}
_distributeGold();
@> uint256 totalGoldAmount = nextAuctionGoldAmount;
nextAuctionGoldAmount = 0;
uint256 epochId = _currentEpochId = _currentEpochId + 1;
if (totalGoldAmount < config.auctionMinimumDistributedGold) { revert LowGoldDistributed(totalGoldAmount); }
EpochInfo storage info = epochs[epochId];
@> info.totalAuctionTokenAmount = totalGoldAmount;
uint128 startTime = info.startTime = uint128(block.timestamp) + config.auctionStartCooldown;
uint128 endTime = info.endTime = startTime + AUCTION_DURATION;
emit AuctionStarted(epochId, msg.sender, startTime, endTime, totalGoldAmount);
}
Note that there is a function called recoverToken(), that allows recovering templeGold tokens.
However, this function only allows recovering tokens from the last non-started auction. Therefore, it is not possible to recover tokens from the last finished auction.
* @notice Recover auction tokens for last but not started auction.
* Any other token which is not Temple Gold can be recovered too at any time
* @dev For recovering Temple Gold, Epoch data is deleted and leftover amount is addedd to nextAuctionGoldAmount.
* so admin should recover total auction amount for epoch if that's the requirement
* @param token Token to recover
* @param to Recipient
* @param amount Amount to auction tokens
*/
function recoverToken(
address token,
address to,
uint256 amount
) external override onlyElevatedAccess {
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (token != address(templeGold)) {
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
IERC20(token).safeTransfer(to, amount);
return;
}
uint256 epochId = _currentEpochId;
EpochInfo storage info = epochs[epochId];
if (info.startTime == 0) { revert InvalidOperation(); }
if (info.isActive()) { revert AuctionActive(); }
if (info.hasEnded()) { revert AuctionEnded(); }
uint256 _totalAuctionTokenAmount = info.totalAuctionTokenAmount;
if (amount > _totalAuctionTokenAmount) { revert CommonEventsAndErrors.InvalidAmount(token, amount); }
delete epochs[epochId];
unchecked {
nextAuctionGoldAmount += _totalAuctionTokenAmount - amount;
}
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
templeGold.safeTransfer(to, amount);
}
Other instances of the issue
The SpiceAuction contract has the same issue.
Impact
The auction tokens allocated to an auction will get stuck in the DaiGoldAuction contract if no bids are placed.
Proof of code
The following test proofs that if an auction has no bids, the allocated auction tokens are not passed to the next auction, and they stay however in the contract balance, with no means of recovering them.
This test can be added in the DaiGoldAuctionTest inside the existing test file protocol/test/forge/templegold/DaiGoldAuction.t.sol.
function test_auctionWithoutBidsLeadsToStuckFunds() public {
_setVestingFactor(templeGold);
skip(1 days);
vm.startPrank(executor);
_startAuction();
uint256 currentEpoch = daiGoldAuction.currentEpoch();
IAuctionBase.EpochInfo memory info = daiGoldAuction.getEpochInfo(currentEpoch);
vm.warp(info.endTime);
vm.startPrank(executor);
_startAuction();
IAuctionBase.EpochInfo memory newInfo = daiGoldAuction.getEpochInfo(daiGoldAuction.currentEpoch());
uint256 nextAuctionGoldAmount = daiGoldAuction.nextAuctionGoldAmount();
assertEq(nextAuctionGoldAmount, 0, "right after starting a new auction, this should be 0");
uint256 pendingToBeClaimedFromPreviousAuctions = 0;
assertEq(
templeGold.balanceOf(address(daiGoldAuction)),
newInfo.totalAuctionTokenAmount + nextAuctionGoldAmount + pendingToBeClaimedFromPreviousAuctions,
"the balance of the contract should be (pendingToBeClaimedFromPreviousAuctions + currentAuction.totalAuctionTokenAmount + nextAuctionGoldAmount)"
);
assertFalse(
templeGold.balanceOf(address(daiGoldAuction)) ==
info.totalAuctionTokenAmount + newInfo.totalAuctionTokenAmount + nextAuctionGoldAmount + pendingToBeClaimedFromPreviousAuctions
);
}
Tools Used
Manual review, foundry for PoC.
Recommendations
If no bids are placed, carry on the allocated auction tokens to the next epoch by adding it to nextAuctionGoldAmount.
function startAuction() external override {
if (auctionStarter != address(0) && msg.sender != auctionStarter) { revert CommonEventsAndErrors.InvalidAccess(); }
EpochInfo storage prevAuctionInfo = epochs[_currentEpochId];
if (!prevAuctionInfo.hasEnded()) { revert CannotStartAuction(); }
AuctionConfig storage config = auctionConfig;
/// @notice last auction end time plus wait period
if (_currentEpochId > 0 && (prevAuctionInfo.endTime + config.auctionsTimeDiff > block.timestamp)) {
revert CannotStartAuction();
}
_distributeGold();
+ if (prevAuctionInfo.totalBidTokenAmount == 0) {
+ nextAuctionGoldAmount += prevAuctionInfo.totalAuctionTokenAmount;
+ }
uint256 totalGoldAmount = nextAuctionGoldAmount;
nextAuctionGoldAmount = 0;
uint256 epochId = _currentEpochId = _currentEpochId + 1;
if (totalGoldAmount < config.auctionMinimumDistributedGold) { revert LowGoldDistributed(totalGoldAmount); }
EpochInfo storage info = epochs[epochId];
info.totalAuctionTokenAmount = totalGoldAmount;
uint128 startTime = info.startTime = uint128(block.timestamp) + config.auctionStartCooldown;
uint128 endTime = info.endTime = startTime + AUCTION_DURATION;
emit AuctionStarted(epochId, msg.sender, startTime, endTime, totalGoldAmount);
}
