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

Overwriting Existing Auctions

0xdevin

Summary

The createAuction function in the SpiceAuctionFactory contract allows for the creation of new auction contracts and stores them in the deployedAuctions mapping. However, the current implementation does not check if an auction already exists for a given spiceToken. As a result, an existing auction can be inadvertently overwritten when creating a new auction, leading to loss of references to the previous auction contract.

Vulnerability Details

No Check for Existing Auction: The line deployedAuctions[pairId] = address(spiceAuction); does not check if an auction already exists for the given pairId. This allows the new auction to overwrite the existing one in the deployedAuctions mapping.

Lack of Safeguards: There is a comment /// @dev not checking pair address exists to allow overwrite in case of a migration, indicating the intentional omission of this check. However, this opens up the possibility of overwriting existing auctions unintentionally.

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuctionFactory.sol#L39C1-L49C1

function createAuction(address spiceToken, string memory name) external override onlyElevatedAccess returns (address) {
if (spiceToken == address(0)) {
revert CommonEventsAndErrors.InvalidAddress();
}
if (spiceToken == templeGold) {
revert CommonEventsAndErrors.InvalidParam();
}
SpiceAuction spiceAuction = new SpiceAuction(templeGold, spiceToken, daoExecutor, name);
bytes32 pairId = _getPairHash(spiceToken);
/// @dev not checking pair address exists to allow overwrite in case of a migration
deployedAuctions[pairId] = address(spiceAuction);
emit AuctionCreated(pairId, address(spiceAuction));
return address(spiceAuction);
}
​

Impact

Loss of Data: The reference to the previous auction is lost, which may contain important state information, history, or funds.

Tools Used

vs code

Recommendations

To prevent unintentional overwriting of existing auctions, the createAuction function should include a check to ensure that an auction does not already exist for the given spiceToken. If an auction already exists, the function should revert or require explicit confirmation to overwrite the existing auction.

  • Calculate a unique pair ID using the _getPairHash function.

  • Check for Existing Auction:

    • Check if an auction already exists in the deployedAuctions mapping for the calculated pair ID.

    • If an auction already exists, revert the transaction with an appropriate error message.

function createAuction(address spiceToken, string memory name) external override onlyElevatedAccess returns (address) {
// Validate input addresses
if (spiceToken == address(0)) {
revert CommonEventsAndErrors.InvalidAddress();
}
if (spiceToken == templeGold) {
revert CommonEventsAndErrors.InvalidParam();
}
​
// Calculate the pair ID for the spice token
bytes32 pairId = _getPairHash(spiceToken);
​
// Check if an auction already exists for the given spice token
require(deployedAuctions[pairId] == address(0), "Auction already exists");
​
// Create a new SpiceAuction contract
SpiceAuction spiceAuction = new SpiceAuction(templeGold, spiceToken, daoExecutor, name);
​
// Store the new auction contract address in the deployedAuctions mapping
deployedAuctions[pairId] = address(spiceAuction);
​
// Emit an event for the creation of the new auction
emit AuctionCreated(pairId, address(spiceAuction));
​
return address(spiceAuction);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Overwriting of existing auctions in `SpiceAuctionFactory`

Support

FAQs

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