Summary

Escrow creation can be front-runned with attack contracts in order to steal funds from buyer.

Vulnerability Details

In create2, we are computing address using creationCode. What creationCode contains is the constructor logic and constructor parameters of smart contracts, not the entire code that is available within contract. Although normally just front-running create2's this way won't be a big issue as mentioned in known issues, because there is a need for token approval to the computed address before creating Escrow: if attacker uses the same constructor logic and constructor parameters in another Escrow implementation that he/she created, they can add malicious functions to this implementation and by front-running newEscrow in their new implementation of EscrowFactory, can steal funds deposited by buyer.

Attack Contracts:

In above contracts there are two main differences:

1- All msg.sender value's in EscrowFactory has changed with parameter "buyer"

2- attack function added to Escrow implementation.

Since adding new functions this way won't change the creationCode, computedAddress won't change.
Since attacker will front-run the buyer using parameter buyer="Caller of the newEscrow", it again won't change the computedAddress because buyer will correspond to "msg.sender" in this codebase. Since buyer is already approved price to the computedAddress, safeTransferFrom will be succesful and Escrow implementation created by attacker will be live with price amount of token inside. After that attacker will be able to call attack and steal funds.

Impact

Direct theft of funds.

Tools Used

Manual Review

Recommendations

Do not send or approve any value to contracts before creation.