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

The `recoverToken()` function uses an incorrect check to verify that the auction has started

StraawHaat

Summary

The recoverToken() function uses an incorrect check to verify that the auction has started. Because of this, it is possible that Temple Gold tokens cannot be recovered.

Vulnerability Details

In DaiGoldAuction.sol we have recoverToken() function:

/**
* @notice Recover auction tokens for last but not started auction.
* Any other token which is not Temple Gold can be recovered too at any time
* @dev For recovering Temple Gold, Epoch data is deleted and leftover amount is addedd to nextAuctionGoldAmount.
* so admin should recover total auction amount for epoch if that's the requirement
* @param token Token to recover
* @param to Recipient
* @param amount Amount to auction tokens
*/
function recoverToken(
address token,
address to,
uint256 amount
) external override onlyElevatedAccess {
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
​
if (token != address(templeGold)) {
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
IERC20(token).safeTransfer(to, amount);
return;
}
​
// auction started but cooldown pending
uint256 epochId = _currentEpochId;
EpochInfo storage info = epochs[epochId];
if (info.startTime == 0) { revert InvalidOperation(); }
if (info.isActive()) { revert AuctionActive(); }
if (info.hasEnded()) { revert AuctionEnded(); }
uint256 _totalAuctionTokenAmount = info.totalAuctionTokenAmount;
if (amount > _totalAuctionTokenAmount) { revert CommonEventsAndErrors.InvalidAmount(token, amount); }
/// @dev Epoch data is deleted and leftover amount is addedd to nextAuctionGoldAmount.
/// so admin should recover total auction amount for epoch if that's the requirement
delete epochs[epochId];
/// @dev `nextAuctionGoldAmount` is set to 0 in `startAuction`.
/// `nextAuctionGoldAmount > 0` if there was a distribution after `auctionStart` called
/// epoch is deleted. so if amount < totalAuctionTokenAmount for epoch, add leftover to next auction amount
unchecked {
nextAuctionGoldAmount += _totalAuctionTokenAmount - amount;
}
​
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
templeGold.safeTransfer(to, amount);
}

This function recover auction tokens for last but not started auction.

From NatSpec comments we can see: "Recover auction tokens for last but not started auction. Any other token which is not Temple Gold can be recovered too at any time".

For recovering Temple Gold we have the following checks:

if (info.startTime == 0) { revert InvalidOperation(); }
if (info.isActive()) { revert AuctionActive(); }
if (info.hasEnded()) { revert AuctionEnded(); }

  • isActive: Checks if the auction is ongoing, which means it has started and has not ended.

We can see that the function uses the wrong check. Instead of isActive() it should use the hasStarted() function.

  • hasStarted: Checks if the auction has begun, regardless of whether it is still ongoing or already ended.

Impact

Due to incorrect check, tokens may not be recoverable.

Tools Used

Visual Studio Code

Recommendations

By changing info.isActive() to info.hasStarted(), the function correctly prevents token recovery if the auction has already started, which is aligned with the intended functionality described in the comment.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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