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

`SpiceAuction.sol`: Auction tokens in `startCooldown` period will be lost when `DAOExecutor` tries to recover these tokens.

superman_i4g

Summary

The Temple team implements a recoverToken function on SpiceAuction.sol to allow DAOExecutor to recover auction tokens (i.e. either spiceToken or templeGold) for an epoch or auction in startCooldown period (i.e not active or ended). However, these auction tokens will be lost when DAOExecutor tries to recover these auction tokens.

Vulnerability Details

According to the codebase or dev, to recover auction tokens in startCooldown period (i.e. auctionStart is called and startCooldown is still pending), the DAOExecutor will use the removeAuctionConfig function to delete the auctionConfig, and the EpochInfo for the epoch in startCooldown period (while decrementing the _currentEpochId) then use the recoverToken function to recover the auction tokens for the epoch.

/// @dev use removeAuctionConfig for case where auctionStart is called and cooldown is still pending
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L250

However, the twist is the recoverToken function only specifies to recover auction tokens that are not in allocation (i.e. not an allocation for an auction). This is because the recoverToken function is also intended to recover auction tokens for users that send to the SpiceAuction contract by mistake therefore these tokens will not be in allocation and can easily be recovered for the users.

uint256 maxRecoverAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[token]);
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L263
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L234-L268

Therefore, the protocol assumes once an auction in startCooldown period has been removed using the removeAuctionConfig function, the exact auction token amount will be out of allocation and will be recoverable with the recoverToken function which is not the case because while removing the auction in removeAuctionConfig function the allocation of the auction was never removed from the global _totalAuctionTokenAllocation (since it's added in the startAuction function for every initialized auction).

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L107-L133
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L171-L173

So each time DAOExecutortries to recover auction tokens for an auction, these amounts will be irrecoverable or lost. To be precise the recoverToken function will always revert because of this check below and these auction amounts will be irrecoverable.

if (amount > maxRecoverAmount) { revert CommonEventsAndErrors.InvalidParam(); }
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L263

Impact

  • Auction tokens to recover are lost (i.e. irrecoverable).

POC

Run command forge test --match-test test_recoverToken_lost_POC in SpiceAuction.t.sol

function test_recoverToken_lost_POC() public {
vm.startPrank(daoExecutor);
​
address _spiceToken = spice.spiceToken();
address _templeGold = address(templeGold);
​
_startAuction(true, true);
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(1);
​
//1. start a dummy auction
​
// auction active
vm.warp(info.startTime);
// some bids
deal(daiToken, alice, 100 ether);
deal(daiToken, bob, 100 ether);
uint256 bidTokenAmount = 10 ether;
vm.startPrank(alice);
IERC20(daiToken).approve(address(spice), type(uint).max);
spice.bid(bidTokenAmount);
vm.startPrank(bob);
IERC20(daiToken).approve(address(spice), type(uint).max);
spice.bid(bidTokenAmount);
// auction ends
vm.warp(info.endTime);
​
// everyone claims
vm.startPrank(alice);
spice.claim(1);
vm.startPrank(bob);
spice.claim(1);
​
// wait period
vm.warp(info.endTime + 2 weeks);
​
//2. real issue demonstration
_startAuction(true, true);
IAuctionBase.EpochInfo memory _info = spice.getEpochInfo(2);
​
// auction in cooldown
vm.warp(_info.startTime - 5);
​
uint256 amountToRecover = _info.totalAuctionTokenAmount;
​
// remove the auction in cooldown to recover auction tokens
vm.startPrank(daoExecutor);
spice.removeAuctionConfig();
​
// recover fails - auction tokens become irrecoverable
vm.expectRevert(abi.encodeWithSelector(CommonEventsAndErrors.InvalidParam.selector));
spice.recoverToken(_templeGold, treasury, amountToRecover);
}

Tools Used

Manual

Recommendations

In the removeAuctionConfig function, remove the info.totalAuctionTokenAmount of the current epoch that will be deleted from the _totalAuctionTokenAllocation of the auction token.

This is a good reference: https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L285-L288

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

missing totalAuctionTokenAllocation deduction in removeAuctionConfig leads to stuck funds

Support

FAQs

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