40,000 USDC
View results
Submission Details
Severity: low
Valid

No check if arbiter is the same address as the seller

Summary

Missing check in the Escrow constructor about the arbiter being the same address as the seller.

Vulnerability Details

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.

Impact

RISK SCALE - LIKELIHOOD
2 - Low probability of an incident occurring.

RISK SCALE - IMPACT
5 - May cause devastating and unrecoverable impact or loss.

Tools Used

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.

Recommendations

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.

Support

FAQs

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