TempleGold

Summary

Temple team implements a recoverToken function on SpiceAuction.sol to allow DAOExecutor to recover arbitary token, spice or auction token (i.e. either spiceToken or templeGold) for users that send to the SpiceAuction contract by mistake. While recovery for any arbitary token is pretty straightforward, recoveries for either spiceToken or templeGold is intended to be only allowed when an epoch or auction is in startCooldown period (i.e. not active or ended). However, the current implementation fails to allow the intended logic i.e. users spiceToken or templeGold tokens cannot be recovered in startCooldown period.

Vulnerability Details

recoverToken function on SpiceAuction.sol allows DAOExecutor to recover a specified amount of a token to a given address. The function differentiates between an arbitary token, auction and spice tokens i.e. either spiceToken or templeGold. While recovery for any arbitrary token is pretty straightforward, recovery for spiceToken or templeGold has additional logic for handling the recovery for users and a removed auction.

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L234-L268

For spiceToken or templeGold, recoverToken function is intended to ensure that the current epoch should not recover these tokens if the epoch has an active auction or already ended i.e should only recover for auctions in auctionStartCooldown period for users who send to the SpiceAuction contract by mistake (although for removed epoch or auction cancelled by DAOExecutor using removeAuctionConfig, recoverToken function can recover the auction token associated to the removed epoch if the current epoch or auction has ended and the next epoch is not set).

While the recoverToken function works fine for removed auctions, the current implementation goes against the intended logic in the case of user recovery i.e DAOExecutor will not be able to recover spiceToken or templeGold for users who send to the SpiceAuction contract by mistake when the current epoch is in startCooldown period. That's because this check below in the recoverToken function will throw a revert when an auction is in startCooldown period and DAOExecutor tries to recover.

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L252

if (!info.hasEnded() && auctionConfigs[epochId+1].duration == 0) { revert RemoveAuctionConfig(); }

If the current epoch is in auctionStartCooldown period, both conditions in the check above (i.e !info.hasEnded() && auctionConfigs[epochId+1].duration == 0) will both return true which will trigger a revert because an auction in auctionStartCooldown period is most definitely not an ended auction neither will auctionConfigs for the next epoch be set (note: auctionConfigs is only set when current epoch has ended and ready to start a new auction).

Therefore, this completely neglects the intended logic thereby will not allow DAOExecutor to recover spiceToken or templeGold token for users who send to the SpiceAuction contract by mistake when the current epoch is in auctionStartCooldown period.

Impact

• Restricts DAOExecutor to recover spiceToken or templeGold for users when the current epoch is in auctionStartCooldown period.

Tools Used

Manual

Recommendations

For both logic to co-exists (i.e recover for removed auction and also for users who send to the SpiceAuction contract by mistake when the current epoch is in auctionStartCooldown period), implement these changes below.