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

Tokens cannot be recovered for `SpiceAuction` even when the auction is in cooldown

shikhar229169

Relevant Github Links

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

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/SpiceAuction.sol#L119-L126

Summary

The tokens in in SpiceAuction is expected to be recovered by the SpiceAuction::recoverToken function for the last but not started auction. For the case when the startAuction is called for an auction and it is currently in cooldown, the function reverts with a message to call RemoveAuctionConfig function.

But removeAuctionConfig just removes the config of the auction and doesn't perform any recovery of the token, as a result of which calling removeAuctionConfig will remove config and tokens that were expected to be recovered will be stuck in the contract and as _totalAuctionTokenAllocation has the value of the token that was expected to be recovered, and the contract evaluate tokens in there as tokens that are allocated to the auction and now there is no possible way to perform recovery after calling removeAuctionConfig, thus resulting in stuck funds.

Vulnerability Details

The vulnerability is present in the recoverToken function of the SpiceAuction contract, where it reverts with a message to call the removeAuctionConfig for the case when an auction is in cooldown and yet to be started.

It was expected for the tokens allocated for an auction currently in cooldown to be recovered via recoverToken, but due to incorrect implementation there is no way to perform recover tokens operation due to above discussed issue, as remove auction config just removes the config and does nothing else.

As removeAuctionConfig performs a reset operation on the epochs and auctionConfigs mapping but as startAuction function was already called so the funds were already allocated in _totalAuctionTokenAllocation mapping as a result of which there is no way for those tokens to be recovered and removeAuctionConfig doesn't perform any updations related to _totalAuctionTokenAllocation.

Impact

Tokens cannot be recovered for the case when the auction is in cooldown and is not started yet.

PoC

Add the below coded PoC in the SpiceAuctionTest contract in the file: test/forge/templegold/SpiceAuction.t.sol

Run the test:

forge test --mt test_AuctionTokenCannotBeRecovered_EvenIfCoolDownIsActive
function test_AuctionTokenCannotBeRecovered_EvenIfCoolDownIsActive() public {
uint160 auctionTokenReward = 100e18;
address recipient = makeAddr("recipient");
// setting up auction config
ISpiceAuction.SpiceAuctionConfig memory _config = ISpiceAuction.SpiceAuctionConfig({
duration: 1 weeks,
waitPeriod: 1,
minimumDistributedAuctionToken: auctionTokenReward,
starter: address(0),
startCooldown: 1 hours,
isTempleGoldAuctionToken: false,
activationMode: ISpiceAuction.ActivationMode.AUCTION_TOKEN_BALANCE,
recipient: recipient
});
​
vm.prank(daoExecutor);
spice.setAuctionConfig(_config);
​
vm.warp(block.timestamp + _config.waitPeriod);
deal(daiToken, address(spice), auctionTokenReward);
spice.startAuction();
​
// now the cooldown period is active for the current epoch
uint256 currentEpoch = spice.currentEpoch();
IAuctionBase.EpochInfo memory _epochInfo = spice.getEpochInfo(currentEpoch);
​
// epoch info set, and auction in cooldown
assert(_epochInfo.startTime > block.timestamp);
​
// Now there is a need to recover the reward token
// the recover token function have a revert statement with the message to call remove auction config
// when startAuction is called, but on cooldown for start
​
uint256 initRecipientBalance = IERC20(daiToken).balanceOf(recipient);
​
vm.startPrank(daoExecutor);
vm.expectRevert(ISpiceAuction.RemoveAuctionConfig.selector);
spice.recoverToken(daiToken, recipient, auctionTokenReward);
​
// now call the remove auction config as guided to recover the tokens
spice.removeAuctionConfig();
vm.stopPrank();
​
// but as we know remove auction config is just used to reset the config parameters.
// thus it is observed that the recoverToken function has implemented things in opposite manner
assertEq(IERC20(daiToken).balanceOf(recipient) - initRecipientBalance, 0);
}

Tools Used

Manual Review, Unit Test in Foundry

Recommendations

  • Updation 1
    Update the recoverToken function to perform the recovery of the tokens for the auction that is in cooldown and yet to be started. Instead of performing a revert for this case recover the tokens to the recipient.
    Perform the following recover operation in the recoverToken function for the case when auction is in cooldown:

+ uint256 amountToRecover = info.totalAuctionTokenAmount;
+ _totalAuctionTokenAllocation[token] -= amountToRecover;
+ IERC20(token).safeTransfer(to, amountToRecover);
+ delete auctionConfigs[id];
+ delete epochs[id];

  • Updation 2
    In removeAuctionConfig update the _totalAuctionTokenAllocation mapping to remove the tokens allocated for the auction that is being removed.

if (!configSetButAuctionStartNotCalled) {
/// @dev unlikely because this is a DAO execution, but avoid deleting old ended auctions
if (info.hasEnded()) { revert AuctionEnded(); }
/// auction was started but cooldown has not passed yet
+ (, address auctionToken) = _getBidAndAuctionTokens(auctionConfigs[id]);
+ _totalAuctionTokenAllocation[auctionToken] -= info.totalAuctionTokenAmount;
delete auctionConfigs[id];
delete epochs[id];
_currentEpochId = id - 1;
emit AuctionConfigRemoved(id, id);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

shikhar229169 Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months 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.