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

Users can bid when the current auction is not active in `DaiGoldAuction`, leading to multiple consequences

0xbrivan2

Vulnerability Details

Bids in DaiGoldAuction for the current auction epoch are allowed only when the epoch is at active state, as confirmed by canDeposit view function:

function canDeposit() external view override returns (bool) {
return epochs[_currentEpochId].isActive();
}

However, if we inspect the bid function we can see that there is no check if the current epoch is at active state:

function bid(uint256 amount) external virtual override onlyWhenLive {
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
// @audit No check if the current epoch is at active state
bidToken.safeTransferFrom(msg.sender, treasury, amount);
​
uint256 epochIdCache = _currentEpochId;
depositors[msg.sender][epochIdCache] += amount;
​
EpochInfo storage info = epochs[epochIdCache];
info.totalBidTokenAmount += amount;
emit Deposit(msg.sender, epochIdCache, amount);
}

As a result, users can bid at cooldown periods or even when the epoch has ended. This will lead to multiple consequences:

  1. Users bidding on the cooldown period will permanently lose their DAI without having the possibility to claim TGLD if a recovery happens for the current epoch. That is because when the recovery happens the epoch data is deleted and the auction token amount is added to the next epoch:

    function recoverToken(
    address token,
    address to,
    uint256 amount
    ) external override onlyElevatedAccess {
    // ...
    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;
    }
    // ...
    }

    So, users who have already made bids for this current epoch (that was deleted because of the recovery) will have neither a way to recover their DAI nor a way to claim TGLD

  2. If a recovery happens on the current epoch, the epoch data is deleted but the _currentEpochId is not incremented. However, users can still bid on the current invalid epoch (that is deleted), and similarly to the first consequence, users have no way to claim TGLD and no way to recover their DAI

  3. Because users can bid after the current epoch ends, they might abuse the system leading other users to claim an unexpected amount of TGLD. That is because, when the current epoch ends, users call claim to claim their TGLD, and the amount to receive depends on the following formula:

    uint256 claimAmount = bidTokenAmount.mulDivRound(info.totalAuctionTokenAmount, info.totalBidTokenAmount, false);

    So, after the auction ends, all the parameters in the formula above should be constant; thus, the claiming amount should be predictable. However, because someone can bid after the current auction ends, a malicious actor can front-run claiming transactions to make a bid and thus increment info.totalBidTokenAmount, leading users to receive an unexpected amount of TGLD.

Impact

The following finding has two impacts as illustrated above:

  1. Permanent lost of DAI without a way to claim TGLD

  2. Malicious actors might abuse the claim system, leading users to receiving unexpected amount of TGLD

Tools Used

Manual Review

Recommendations

Consider allowing bids only when the current epoch is active. Below is a suggestion for an updated code of the bid function:

function bid(uint256 amount) external virtual override onlyWhenLive {
+ uint256 epochIdCache = _currentEpochId;
+ EpochInfo storage info = epochs[epochIdCache];
+ if(!info.isActive()) { revert CannotDeposit(); }
​
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
​
bidToken.safeTransferFrom(msg.sender, treasury, amount);
​
- uint256 epochIdCache = _currentEpochId;
depositors[msg.sender][epochIdCache] += amount;
​
- EpochInfo storage info = epochs[epochIdCache];
info.totalBidTokenAmount += amount;
emit Deposit(msg.sender, epochIdCache, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
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.