TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Auction tokens cannot be recovered for the first ever spice auction

Trident Audits

Summary

The SpiceAuction::recoverToken() function is used to recover the auction tokens (either temple gold or a spice token) for an auction whose config is set, but hasn't started yet. However, for auction id 1 (and epoch id 0, since auction hasn't started yet), tokens can never be recovered if the dao wanted to do so.

NOTE
Auction id isn't a state variable. It is used to better represent the latest auction number. Auction config is set for the _currentEpochId + 1 index, and the _currentEpochId variable lags behind the auction id by 1 until the auction starts. Then the _currentEpochId is incremented, and epoch info is set.

Vulnerability Details

Consider the following code segment from SpiceAuction::recoverToken() function,

function recoverToken(address token, address to, uint256 amount) external override onlyDAOExecutor {
// ...
​
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.hasEnded() && auctionConfigs[epochId + 1].duration == 0) revert RemoveAuctionConfig();
​
// ...
}

For the first spice auction which hasn't started yet (auction id 1 and epoch id 0), the epoch info hasn't been set (it is set once the SpiceAuction::startAuction() function is called). Thus, L#249 accesses a non-existent epoch info (all fields in the struct hold value 0) and the function reverts at L#251.

Additionally, auction tokens cannot be recovered during the cooldown period. This will revert at L#252.

Impact

The dao executor won't be able to recover tokens for the first ever spice auction. This is essentially a DOS type of vulnerability.

Proof Of Concept

Add the following test to SpiceAuctionTest contract in ./protocol/test/forge/templegold/SpiceAuction.t.sol,

function testRecoverTokenFailsForFirstAuction() public {
ISpiceAuction.SpiceAuctionConfig memory config = _getAuctionConfig();
vm.startPrank(daoExecutor);
spice.setAuctionConfig(config);
vm.stopPrank();
​
address auctionToken = config.isTempleGoldAuctionToken ? address(templeGold) : spice.spiceToken();
dealAdditional(IERC20(auctionToken), address(spice), 100 ether);
​
vm.prank(daoExecutor);
vm.expectRevert(ISpiceAuction.InvalidConfigOperation.selector);
spice.recoverToken(auctionToken, daoExecutor, 100 ether);
}

The test passes, with the following logs,

Ran 1 test for test/SpiceAuction.t.sol:SpiceAuctionTest
[PASS] testRecoverTokenFailsForFirstAuction() (gas: 396290)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.06s (2.92ms CPU time)
​
Ran 1 test suite in 2.08s (2.06s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual review and Foundry for writing POC and tests.

Recommended Mitigation

Make the following changes to SpiceAuction::recoverToken() functions,

function recoverToken(address token, address to, uint256 amount) external override onlyDAOExecutor {
// ...
​
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.hasEnded() && auctionConfigs[epochId+1].duration == 0) { revert RemoveAuctionConfig(); }
+ if (epochId != 0) {
+ if (info.startTime == 0) revert InvalidConfigOperation();
+ if (!info.hasEnded() && auctionConfigs[epochId + 1].duration == 0) revert RemoveAuctionConfig();
+ }
​
// ...
}

Now let's unit test the changes,

function testRecoverTokenForFirstAuction() public {
ISpiceAuction.SpiceAuctionConfig memory config = _getAuctionConfig();
vm.startPrank(daoExecutor);
spice.setAuctionConfig(config);
vm.stopPrank();
​
address auctionToken = config.isTempleGoldAuctionToken ? address(templeGold) : spice.spiceToken();
dealAdditional(IERC20(auctionToken), address(spice), 100 ether);
​
vm.prank(daoExecutor);
spice.recoverToken(auctionToken, daoExecutor, 100 ether);
​
assertEq(IERC20(auctionToken).balanceOf(daoExecutor), 100 ether);
}

The test passes with the following logs,

Ran 1 test for test/SpiceAuction.t.sol:SpiceAuctionTest
[PASS] testRecoverTokenForFirstAuction() (gas: 424640)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.59s (3.46ms CPU time)
​
Ran 1 test suite in 1.61s (1.59s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`removeAuctionConfig` can't remove the first added `SpiceAuctionConfig` which in the end leads to inability to recover the funds associated to that auction

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.