Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Frontrunning/Dos possible with contest creation in ProxyFactory.sol and other issues

0xrizwan

Summary

Frontrunning possible with contest creation in ProxyFactory.sol

Vulnerability Details

Impact

Issue 1:

In ProxyFactory.sol, setContest() is used to set the new contest which can be accessed by owner only.

​
function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
public
onlyOwner
{
if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}
>> bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = closeTime;
emit SetContest(organizer, contestId, closeTime, implementation);
}

Here the salt is a bytes32 value that is used in contest creation call i.e setContest() by the owner. It has used _calculateSalt() function to get the bytes32 salt value,

​
function _calculateSalt(address organizer, bytes32 contestId, address implementation)
internal
pure
returns (bytes32)
{
>> return keccak256(abi.encode(organizer, contestId, implementation));
}

Notice here, it has only used arguments such as organizer, contestId, implementation for calculating the salt, however such implementation is susceptible to front runnning. The frontrunning can occur in the following way:

  1. When the contest creation is called by owner. An attacker monitors the mempool for pending transactions that involve cloning a contract with a provided "salt".

  2. Upon spotting such a transaction, the attacker extracts the "salt" value.

  3. The attacker quickly submits their own transaction with a higher gas price, attempting to clone the contract with the same "salt" before the original transaction is mined.

  4. If the transaction got successful, the attacker's transaction is mined first, and the contract clone is created at the expected address.

  5. The original transaction will likely fail, as the contract with the expected address has already been deployed.

Issue 2:
In similar way, attacker can front run the creation of getProxyAddress(),

​
function getProxyAddress(bytes32 salt, address implementation) public view returns (address proxy) {
bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(uint160(implementation)));
>> bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, keccak256(code)));
proxy = address(uint160(uint256(hash)));
}

Here salt and implementation is passed as an argument but as mentioned above this can also be front run in similar way. Check the recommendations to mitigate this issue.

Issue 3:
As discussed in Issue 1 about _calculateSalt(). This function is also used in _deployProxy().

​
function _deployProxy(address organizer, bytes32 contestId, address implementation) internal returns (address) {
>> bytes32 salt = _calculateSalt(organizer, contestId, implementation);
address proxy = address(new Proxy{salt: salt}(implementation));
return proxy;
}

Since this has used _calculateSalt() as an internal function for which the front running prevention is already taken care in _calculateSalt() in recommendation. _deployProxy() has used create2 for contract creation with salt is provided as an argument. Create2 has common issue of front running or DOS however with the provided recommendation it is already prevented.

Tools Used

Manual review

Recommendations

Use a salt that includes the msg.sender. That way it is not possible to front-run the transaction.

​
function _calculateSalt(address organizer, bytes32 contestId, address implementation)
internal
pure
returns (bytes32)
{
- return keccak256(abi.encode(organizer, contestId, implementation));
+ return keccak256(abi.encode(organizer, contestId, implementation, msg.sender));
}
​
​
​
// some code
​
function getProxyAddress(bytes32 salt, address implementation) public view returns (address proxy) {
+ bytes32 _salt = keccak256(abi.encode(salt, msg.sender);
bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(uint160(implementation)));
- bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, keccak256(code)));
+ bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), _salt, keccak256(code)));
proxy = address(uint160(uint256(hash)));
}

Support

FAQs

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