The EscrowFactory:newEscrow()
function intakes 6 parameters including Salt If the Buyer deploys a second escrow contract with the same parameters and salt has been reused the creation of the escrow contract will be reverted.
A defi protocol buys an audit from Cyfrin audit and creates an escrow factory with the following parameters. EscrowFactory:newEscrow(100,0x1,address(cyfrin),0x2,10,0x0001)
Address 0x32..
will be computed and an escrow contract will be created on address 0x32..
The Same defi protocol wishes to give a second audit with the same price to Cyfrin. The Buyer(Defi Protocol) calls newEscrow()
on EscrowFactory as the same parameters on the first audit as the following params EscrowFactory:newEscrow(100,0x1,address(cyfrin),0x2,10,0x0001)
The Same Address 0x32..
will be computed but the creation of the escrow contract on 0x32..
will be reverted
Run this Foundry Test code inside the test/unit/
folder of https://github.com/Cyfrin/2023-07-escrow/
Repo:
forge test -vv --mp test/unit/Escrow-Poc.t.sol
The Foundry test code contains vm.expectRevert()
where the test case passes means The user creates escrow with same params and salt will be reverted on the creation of the second deployment
Hence, the creation of a second deployment of escrow will be reverted, if the user uses the same Salt and params.
Manual Review, Foundry
I recommend this fix:
There should be a public array for Storing the Salt value of the user.
User input of Salt is ignored Hence it also solves the known issue of FrontRunning
Salt Array should be increment on each call hence it's used on the computeEscrowAddress()
function.
On Creation by new Escrow{salt: bytes32(salt)}
the unit salt should be converted to bytes32(salt).
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.