Summary
The view function DaiGoldAuction.isCurrentEpochEnded() will return true for non-started epochs when recoverToken() is called to recover templeGold tokens.
Vulnerability Details
The function hasEnded() from the EpochLib library returns true when info.endTime == 0.
library EpochLib {
function hasEnded(IAuctionBase.EpochInfo storage info) internal view returns (bool) {
return info.endTime <= block.timestamp;
}
This is used (among other places) by the view DaiGoldAuction.isCurrentEpochEnded() function.
contract DaiGoldAuction {
function isCurrentEpochEnded() external view override returns (bool) {
return epochs[_currentEpochId].hasEnded();
}
Moreover, when the function DaiGoldAuction.recoverToken() is called to recover templeGold tokens, the remaining non-recovered templeGold tokens are added to nextAuctionGoldAmount, and then the epoch info of the current epoch is deleted:
* @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;
}
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); }
@> delete epochs[epochId];
unchecked {
nextAuctionGoldAmount += _totalAuctionTokenAmount - amount;
}
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
templeGold.safeTransfer(to, amount);
}
This means that when recoverToken() is called to recover templeGold tokens, since the epoch info is deleted, the view function isCurrentEpochEnded() will return true even if the auction has not been started yet.
Impact
The view function DaiGoldAuction.isCurrentEpochEnded() will return wrong information after recovering templeGold tokens from the DaiGoldAuction.
Tools Used
Manual review.
Recommendations
In case usages of EpochLib.hasEnded() expect true even if endTime==0, I would suggest to only change the view function behavior:
/**
* @notice Check if current epoch ended. That is, if current auction ended.
* @return Bool if epoch ended
*/
function isCurrentEpochEnded() external view override returns (bool) {
+ if (epochs[_currentEpochId].endTime == 0) return false;
return epochs[_currentEpochId].hasEnded();
}
