Summary
The TempleDAO allows the DAO executor to withdraw tokens from auctions that have no bids and update the necessary states. However, it does not mark the auction as withdrawn, which can cause a DoS, if multiple withdrawal calls are executed on a single auction.
Vulnerability Details
The Temple DAO with withdraw the auction tokens which were transferred for auction biding and the auction did not receive any bid from users. So in this case it will allow the DAOExecutor to withdraw these tokens from spiceAuction contract via calling recoverAuctionTokenForZeroBidAuction:
function recoverAuctionTokenForZeroBidAuction(uint256 epochId, address to) external override onlyDAOExecutor {
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (epochId > _currentEpochId) { revert InvalidEpoch(); }
EpochInfo storage epochInfo = epochs[epochId];
if (!epochInfo.hasEnded()) { revert AuctionActive(); }
if (epochInfo.totalBidTokenAmount > 0 ) { revert InvalidOperation(); }
SpiceAuctionConfig storage config = auctionConfigs[epochId];
(, address auctionToken) = _getBidAndAuctionTokens(config);
uint256 amount = epochInfo.totalAuctionTokenAmount;
_totalAuctionTokenAllocation[auctionToken] -= amount;
emit CommonEventsAndErrors.TokenRecovered(to, auctionToken, amount);
IERC20(auctionToken).safeTransfer(to, amount);
}
POC:
add import import { IERC20Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol"; inside SpiceAuction.t.sol file.
function test_recoverAuctionTokenForZeroBidAuction() public {
vm.startPrank(daoExecutor);
vm.expectRevert(abi.encodeWithSelector(CommonEventsAndErrors.InvalidAddress.selector));
spice.recoverAuctionTokenForZeroBidAuction(0, address(0));
vm.expectRevert(abi.encodeWithSelector(IAuctionBase.InvalidEpoch.selector));
spice.recoverAuctionTokenForZeroBidAuction(1, alice);
_startAuction(true, true);
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(1);
vm.startPrank(daoExecutor);
vm.warp(info.startTime);
vm.expectRevert(abi.encodeWithSelector(IAuctionBase.AuctionActive.selector));
spice.recoverAuctionTokenForZeroBidAuction(1, alice);
vm.startPrank(bob);
uint256 bidAmount = 10 ether;
deal(daiToken, bob, bidAmount);
IERC20(daiToken).approve(address(spice), type(uint).max);
spice.bid(bidAmount);
uint256 epochOneTotalAuctionTokenAmount = info.totalAuctionTokenAmount;
ISpiceAuction.SpiceAuctionConfig memory _config = spice.getAuctionConfig(1);
vm.warp(info.endTime + _config.waitPeriod);
vm.startPrank(daoExecutor);
vm.expectRevert(abi.encodeWithSelector(IAuctionBase.InvalidOperation.selector));
spice.recoverAuctionTokenForZeroBidAuction(1, alice);
_startAuction(true, true);
info = spice.getEpochInfo(2);
_config = spice.getAuctionConfig(2);
vm.warp(info.endTime + _config.waitPeriod);
address auctionToken = spice.getAuctionTokenForCurrentEpoch();
uint256 auctionTokenBalance = IERC20(auctionToken).balanceOf(address(spice));
uint256 auctionTokenAmount = info.totalAuctionTokenAmount;
uint256 aliceBalance = IERC20(auctionToken).balanceOf(alice);
vm.startPrank(daoExecutor);
vm.expectEmit(address(spice));
emit TokenRecovered(alice, auctionToken, auctionTokenAmount);
spice.recoverAuctionTokenForZeroBidAuction(2, alice);
spice.recoverAuctionTokenForZeroBidAuction(2, alice);
assertEq(IERC20(auctionToken).balanceOf(alice), aliceBalance + (auctionTokenAmount*2));
vm.startPrank(bob);
uint256 bobBalance = IERC20(auctionToken).balanceOf(bob);
console.logAddress(address(auctionToken));
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, address(0xffD4505B3452Dc22f8473616d50503bA9E1710Ac), 0, 100000000000000000000));
spice.claim(1);
assertEq(IERC20(auctionToken).balanceOf(bob), bobBalance + 0);
}
run with command : forge test --mt test_recoverAuctionTokenForZeroBidAuction
Impact
If the recoverAuctionTokenForZeroBidAuction called more than once it will create a DoS for other user and Users of other auctions with the same auction tokens will be unable to claim their tokens.
Tools Used
Manual Review
Recommendation
Add boolean state variable to EpochInfo which will be set to true if recoverAuctionTokenForZeroBidAuction has called first time , also add condition which will check for if auction tokens have already recovered.
struct EpochInfo {
/// @notice Start time for epoch
uint128 startTime;
/// @notice End time for epoch
uint128 endTime;
/// @notice Total amount of bid token deposited
uint256 totalBidTokenAmount;
/// @notice Total amount of auction tokens to distribute. Constant value
uint256 totalAuctionTokenAmount;
++ bool isRecovered;
}
// inside spiceAuction::recoverAuctionTokenForZeroBidAuction function add following changes and also add `AlreadyRecovered` inside `ISpiceAuction` interface.
EpochInfo storage epochInfo = epochs[epochId];
if (!epochInfo.hasEnded()) {
revert AuctionActive();
}
++ if (epochInfo.isRecovered) {
++ revert AlreadyRecovered();
}
// bid token amount for epoch has to be 0
if (epochInfo.totalBidTokenAmount > 0) {
revert InvalidOperation();
}
SpiceAuctionConfig storage config = auctionConfigs[epochId];
(, address auctionToken) = _getBidAndAuctionTokens(config);
uint256 amount = epochInfo.totalAuctionTokenAmount;
_totalAuctionTokenAllocation[auctionToken] -= amount;
++ epochInfo.isRecovered = true;
