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 over 1 year 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.

Give us feedback!