Summary
In the SpiceAuction.sol file, The documentation states, "Recover auction tokens for the last auction that has not yet started." However, we can recover tokens even if the auction is currently active.
Vulnerability Details
In the SpiceAuction.sol file, there is recoverToken function.
* @notice Recover auction tokens for last but not started auction
* @param token Token to recover
* @param to Recipient
* @param amount Amount to auction tokens
*/
function recoverToken(
address token,
address to,
uint256 amount
) external override onlyDAOExecutor {
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (token != spiceToken && token != 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 InvalidConfigOperation(); }
if (!info.hasEnded() && auctionConfigs[epochId+1].duration == 0) { revert RemoveAuctionConfig(); }
On line 25, we didn't check if info.isActive(). Therefore, if the current auction is active and the next auction's configuration is set (which can be done during the cooldown time of the current auction), we can recover the token.
Impact
The token recovery process can be run even if the auction is active.
POC
I added test code to demonstrate how we can process the recovery token even when the auction is active.
The test code below should fail, but it run as success.
function test_recoverToken_with_active_spice() public {
address _spiceToken = spice.spiceToken();
uint256 recoverAmount = 1 ether;
_startAuction(true, true);
dealAdditional(IERC20(daiToken), address(spice), recoverAmount);
vm.startPrank(daoExecutor);
spice.setAuctionConfig(_getAuctionConfig());
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(1);
vm.warp(info.endTime - 1);
assertEq(true, info.startTime <= block.timestamp && block.timestamp < info.endTime);
emit TokenRecovered(alice, _spiceToken, recoverAmount);
spice.recoverToken(_spiceToken, alice, recoverAmount);
}
Tools Used
Manual Review
Recommendations
In the SpiceAuction.sol file L252, we need to add to checking info.isActive() code.
/**
* @notice Recover auction tokens for last but not started auction
* @param token Token to recover
* @param to Recipient
* @param amount Amount to auction tokens
*/
function recoverToken(
address token,
address to,
uint256 amount
) external override onlyDAOExecutor {
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (token != spiceToken && token != templeGold) {
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
IERC20(token).safeTransfer(to, amount);
return;
}
uint256 epochId = _currentEpochId;
EpochInfo storage info = epochs[epochId];
/// @dev use `removeAuctionConfig` for case where `auctionStart` is called and cooldown is still pending
if (info.startTime == 0) { revert InvalidConfigOperation(); }
+ if (info.isActive()) { revert AuctionActive(); }
if (!info.hasEnded() && auctionConfigs[epochId+1].duration == 0) { revert RemoveAuctionConfig(); }
