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

Overwrite Risk in `deployedAuctions` Mapping

kevinkkien

Summary

The SpiceAuctionFactory contract's deployedAuctions mapping does not prevent overwriting existing auction addresses. This could lead to the loss of access to previous auctions and potential manipulation of auction data.

Vulnerability Details

The createAuction function in SpiceAuctionFactory allows the creation of new SpiceAuction contracts. However, it does not check if an auction for the given token pair already exists in the deployedAuctions mapping. This means that if createAuction is called again with the same token pair, the existing auction address will be overwritten with the new one, effectively making the previous auction inaccessible.

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/SpiceAuctionFactory.sol#L39-L48

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

Users who participated in previous auctions may lose the ability to claim their rewards or view auction details if the corresponding auction address is overwritten.

Tools Used

Manual Review

Recommendations

Before adding a new auction address to the deployedAuctions mapping, check if an entry for the given token pair already exists. If so, either revert the transaction or provide an option to update the existing auction address

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(); }
// Check if auction already exists for the token pair
bytes32 pairId = _getPairHash(spiceToken);
if (deployedAuctions[pairId] != address(0)) {
revert AuctionAlreadyExists(pairId); // Report an error if it exists
}
​
SpiceAuction spiceAuction = new SpiceAuction(templeGold, spiceToken, daoExecutor, name);
​
deployedAuctions[pairId] = address(spiceAuction);
emit AuctionCreated(pairId, address(spiceAuction));
return address(spiceAuction);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months 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.