TempleGold

Summary

In spiceAuction contract, first daoExecutor will setAuctionConfig where the AuctionConfig details are stored inside mapping then to start that auction of that auctionconfig he calls startAcution(), which set the epoch data in epoch mapping and transfer balance amount to contract for auction to start and set global variable`, to make explanation easier lets assume we are calling contract for first time so there is not claimable rewards at this point of time, the issue arise when the we DaoExecutor try to removeConfig under coolDown period. they money which sent to contract is lost

Vulnerability Details

1. first DaoEXecutor call the Auctionconfig function, its the first time we are calling Spice auction contract(means 1st iteration), and parallely we can't set config for 2 auction becuase setAuctionConfig will check previous epoch is ended or not, then StartAuction function is called where auction amount is set for that epoch and data for epoch is set which is uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[auctionToken]; = = 0 becuase we calling startAuction for first , uint256 balance = IERC20(auctionToken).balanceOf(address(this)); = 100 suppose we decided auction amount to be 100 so transferred 100 tokens direclty to contract, uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]); = epochAuctionTokenAmount == 100 because totalAuctionTokenAllocation = 0 and _claimedAuctionTokens = 0 because as are we interacting first time nobody has claimed yet so 0 , so epochAuctionTokenAmount = 100 , now in last line global variable is increased to 100, _totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + epochAuctionTokenAmount; , now there was a cool down and we remove the config before cooldown end while performing 2nd iteration uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[auctionToken]; = 100 because it been set in 1st iteration and uint256 balance = IERC20(auctionToken).balanceOf(address(this)); == 100 old balance we are not transferring new amount by assuming it will use old money, but while calculating epoch uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]); 100 - (100-0) = 0 the epochAuctionTokenAMount is 0 we didn't use old balance.

   and at last _totalAuctionTokenAllocation[auctionToken] = totalAuctionTokenAllocation + epochAuctionTokenAmount; = 100 + 0 = 100 still 100

2. last thing uint256 epochAuctionTokenAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[auctionToken]); this shd also be 100, but its 0

   100-(100-0) = 0

   then info.totalAuctionTokenAmount = epochAuctionTokenAmount; this epoch info will also be set to 0

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

Impact

The money which is sent to contract is lost for partcular epoch which DaoExecutor has remove, any there is no other way to remove the config and epoch data other than this function under cool down period, which don't handle the asset transfer poperly.

so they should transfer back the amount which was sent to that epoch back to the recipent info.totalAuctionTokenAmount= epochAuctionTokenAmount

Tools Used

Manual review

Recommendations

Protocol should re-implement the design choice for cool or they can just transfer back the info.totalAuctionTokenAmount= epochAuctionTokenAmountback to recipent similar to recoverAuctionTokenForZeroBidAuction