The EscrowFactory#newEscrow() function creates a new escrow contract with the provided parameters: price, token contract, seller, arbiter, arbiter fee, salt.
Computes the address: calls computeEscrowAddress() function to determine what the address of the newly created escrow will be.
Transfers tokens to the computed address.
Creates a new escrow contract using the same parameters and a given salt.
Checks that the computed address matches the created escrow contract's address, and reverts if not.
On the other hand the helper computeEscrowAddress() function computes the deterministic address where the escrow contract will be deployed, based on the provided parameters and the rules for salted contract creations (CREATE2).
Vulnerability Details
The vulnerability occurs since the computeEscrowAddress() computation is deterministic and depends on the provided parameters: price, token contract, seller, arbiter, arbiter fee, salt.
Impact
There are several potential issues:
Potential Front-Running: An Attacker who knows the parameters used for the deterministic address calculation can frontrun the creation TX with their own creation request, with the same parameters. This would create the exact address created by the CREATE2 call, since the parameters and therefore the final salt will be the same. When the victim's transaction would be executed, the address is non-empty so the EVM would reject its creation. This would result in a bad UX for a user, who thinks the creation did not succeed. The result contract would still be usable, but would be hard to track as it was created in another TX.
Impact on Payload Execution: Front-running could lead to tokens being transferred to a malicious contract, and the subsequent creation of the Escrow contract would revert.
Lack of Pre-Deployment Check: Neither the provided code snippet nor the description includes a pre-deployment check to see if a contract has already been deployed at the expected address.
Tools Used
Manual Review
Recommendations
Consider checking for the contracts in advance at the predicted addresses in EscrowFactory#newEscrow() and not deploying if they are already there. In this case, consider also checking that the contract’s code is as expected
Contracts can be created before execution and the Attacker can make a signer gate creation fail
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/EscrowFactory.sol#L20-L53
Summary
The
EscrowFactory#newEscrow()function creates a new escrow contract with the provided parameters: price, token contract, seller, arbiter, arbiter fee, salt.computeEscrowAddress()function to determine what the address of the newly created escrow will be.On the other hand the helper
computeEscrowAddress()function computes the deterministic address where the escrow contract will be deployed, based on the provided parameters and the rules for salted contract creations (CREATE2).Vulnerability Details
The vulnerability occurs since the
computeEscrowAddress()computation is deterministic and depends on the provided parameters: price, token contract, seller, arbiter, arbiter fee, salt.Impact
There are several potential issues:
Potential Front-Running: An Attacker who knows the parameters used for the deterministic address calculation can frontrun the creation TX with their own creation request, with the same parameters. This would create the exact address created by the CREATE2 call, since the parameters and therefore the final salt will be the same. When the victim's transaction would be executed, the address is non-empty so the EVM would reject its creation. This would result in a bad UX for a user, who thinks the creation did not succeed. The result contract would still be usable, but would be hard to track as it was created in another TX.
Impact on Payload Execution: Front-running could lead to tokens being transferred to a malicious contract, and the subsequent creation of the Escrow contract would revert.
Lack of Pre-Deployment Check: Neither the provided code snippet nor the description includes a pre-deployment check to see if a contract has already been deployed at the expected address.
Tools Used
Manual Review
Recommendations
Consider checking for the contracts in advance at the predicted addresses in
EscrowFactory#newEscrow()and not deploying if they are already there. In this case, consider also checking that the contract’s code is as expected