Relevant GitHub Links
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/SpiceAuction.sol#L171-L173
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/SpiceAuction.sol#L123-L125
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/SpiceAuction.sol#L261
Summary
SpiceAuction::removeAuctionConfig can cancel AuctionConfigs in two states: those yet to begin and those in the cooldown period. Once an auction enters cooldown, it means it has been initiated via SpiceAuction::startAuction. During initiation, the contract allocates the auction amount to SpiceAuction::_totalAuctionTokenAllocation[auctionToken], considering these tokens as allocated. When the protocol removes an auction in cooldown, it fails to restore this portion of tokens, resulting in both users and the project team being unable to withdraw these tokens, effectively locking them in the protocol.
Vulnerability Details
In SpiceAuction::startAuction, once an auction is initiated, the auctioned tokens are considered allocated:
function startAuction() external override {
...
(,address auctionToken) = _getBidAndAuctionTokens(config);
@> uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[auctionToken];
@> uint256 balance = IERC20(auctionToken).balanceOf(address(this));
@> uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]);
if (config.activationMode == ActivationMode.AUCTION_TOKEN_BALANCE) {
if (config.minimumDistributedAuctionToken == 0) { revert MissingAuctionTokenConfig(); }
}
if (epochAuctionTokenAmount < config.minimumDistributedAuctionToken) { revert NotEnoughAuctionTokens(); }
epochId = _currentEpochId = _currentEpochId + 1;
EpochInfo storage info = epochs[epochId];
uint128 startTime = info.startTime = uint128(block.timestamp) + config.startCooldown;
uint128 endTime = info.endTime = startTime + config.duration;
@> info.totalAuctionTokenAmount = epochAuctionTokenAmount;
@> _totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + epochAuctionTokenAmount;
emit AuctionStarted(epochId, msg.sender, startTime, endTime, epochAuctionTokenAmount);
}
However, in SpiceAuction::removeAuctionConfig, when handling auctions in cooldown, it merely deletes the corresponding info and config without restoring the allocated tokens to an unallocated state:
function removeAuctionConfig() external override onlyDAOExecutor {
...
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);
} ...
}
As the info and config related to this auction are deleted, recoverAuctionTokenForZeroBidAuction() cannot be used, and recoverToken() excludes these tokens from recoverable range as they are considered allocated:
function recoverToken(
address token,
address to,
uint256 amount
) external override onlyDAOExecutor {
...
@> uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[token];
@> uint256 balance = IERC20(token).balanceOf(address(this));
@> uint256 maxRecoverAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[token]);
@> if (amount > maxRecoverAmount) { revert CommonEventsAndErrors.InvalidParam(); }
IERC20(token).safeTransfer(to, amount);
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
}
This ultimately results in the protocol being unable to further process these tokens, locking them within the protocol.
POC
This POC demonstrates the following steps:
-
Initiate an auction with 100 ether and set the time to be within the cooldown period.
At this point, the protocol receives 100 ether, the auction amount is 100 ether, and these 100 ether tokens are considered allocated.
-
Cancel the auction.
The protocol still holds a balance of 100 ether, the auction information is canceled, but these 100 ether tokens are still considered allocated.
-
The protocol initiates another 100 ether auction and lets it complete to pass subsequent recoverToken checks.
Now the protocol has a balance of 200 ether, with 200 ether considered as allocated tokens.
We know that only 100 ether of tokens were normally auctioned, and the remaining 100 ether needs to be extracted.
-
Attempt to extract the remaining 100 ether using recoverToken, but find there's no withdrawable balance.
This POC illustrates how tokens can become locked in the contract due to the vulnerability in the removeAuctionConfig().
Place the following code in SpiceAuction.t.sol::SpiceAuctionTest:
function test_removeAuctionConfigInCoolDownAndTokenisLocked() public {
ISpiceAuction.SpiceAuctionConfig memory _config = _startAuction(true, true);
address auctionToken = spice.getAuctionTokenForCurrentEpoch();
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(spice.currentEpoch());
vm.warp(info.startTime - 10 seconds);
assertEq(IERC20(auctionToken).balanceOf(address(spice)), 100 ether);
assertEq(info.totalAuctionTokenAmount, 100 ether);
assertEq(spice.getTotalAuctionTokenAmount(auctionToken), 100 ether);
vm.startPrank(daoExecutor);
spice.removeAuctionConfig();
emit log("---------------removed-------------");
assertEq(IERC20(auctionToken).balanceOf(address(spice)), 100 ether);
assertEq(spice.getTotalAuctionTokenAmount(auctionToken), 100 ether);
_config = _startAuction(true, true);
emit log("-----------------start again------------");
info = spice.getEpochInfo(spice.currentEpoch());
assertEq(IERC20(auctionToken).balanceOf(address(spice)), 200 ether);
assertEq(info.totalAuctionTokenAmount, 100 ether);
assertEq(spice.getTotalAuctionTokenAmount(auctionToken), 200 ether);
vm.warp(info.endTime + _config.waitPeriod);
vm.startPrank(daoExecutor);
vm.expectRevert(abi.encodeWithSelector(CommonEventsAndErrors.InvalidParam.selector));
spice.recoverToken(auctionToken, address(1), 1);
}
Impact
The affected tokens will be permanently locked in the contract, causing irreversible financial loss to the project team.
Tools Used
Manual Review.
Recommendations
Before deleting auction-related information, restore the allocated tokens to an unallocated state. For enhanced security, consider using recoverToken() within the function to return these tokens to the token issuer:
- function removeAuctionConfig() external override onlyDAOExecutor {
+ function removeAuctionConfig(address _to) external override onlyDAOExecutor {
...
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
+ (, address auctionToken) = _getBidAndAuctionTokens(auctionConfigs[id]);
+ _totalAuctionTokenAllocation[auctionToken] -= info.totalAuctionTokenAmount;
+ recoverToken(auctionToken, _to, info.totalAuctionTokenAmount); // Optional
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, id);
}
...
}
