Missing check in the Escrow constructor about the arbiter being the same address as the seller.
If a buyer accidentally declares the address of the arbiter as the same as the seller, this can drain all the funds that the buyer deposited into the Escrow. And the buyer doesn't have any chance to cancel the escrow or change the arbiter.
RISK SCALE - LIKELIHOOD
2 - Low probability of an incident occurring.
RISK SCALE - IMPACT
5 - May cause devastating and unrecoverable impact or loss.
Manual review
Foundry testing
You can check the test that executes the malicious code in test/audit-tests/UnexpectedBehaviours.t.sol
The process for the issue is the following:
1 - The buyer accidently creates the Escrow with the same address for the seller and the arbiter
2 - The seller notices about the buyer's mistake and calls the function initiateDispute()
3 - Once the contract is in Disputed state the seller calls resolveDispute(0) giving 0 as the amount to award for the buyer, receiving the arbiterFee + the remaining amount. In other words, the seller receives all the funds deposited by the buyer.
1 - Add a check in the constructor to disable the seller to also be the arbiter like this:
if (seller == arbiter) revert Escrow__ArbiterCanNotBeTheSeller();
2 - Add a cancelEscrow function that would enable the buyer to cancel the escrow once he noticed that he made the mistake and return the funds to the buyer. It would be also necessary to add an extra state. For example, an "Accepted" state that would mean that the seller has accepted the escrow trade and the cancelEscrow function would only be callable in the Created state just before Accepted state.
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.