CodeHawks Escrow Contract - Competition Details

Summary

The current Escrow contract implementation allows anyone to be the arbitor even the 0x0 address this in turn permits a scenario where the buyer also acts as the arbiter. This presents a potential vulnerability where a buyer, acting in bad faith, can initiate a dispute and manipulate its resolution in their favor, undermining the escrow's purpose of ensuring fair transactions.

Vulnerability Details

In the Escrow.sol contract, it is possible for a buyer to set themselves as the arbitor during contract initialization, as suggested by this lines 34 & 62 in the EscrowFactory.sol contract

As seen the contract does not have an explicit check to prevent the buyer from being the arbiter, infact the arbitor could be set as the arbitor but thats a different topic . Note that this lack of restriction is critical during dispute resolution in the resolveDispute(uint256 buyerAward) function. The buyer, acting as the arbiter, can award the entire balance to themselves regardless of whether the seller fulfilled their obligations.

So even if a seller did a good job and calls initiateDispute() due to not receiving funds, the arbitor is not a third party so it could as well mean that the buyer has the overall final decision on how it turns out, which breaks contract's intended logic since the arbitor is supposed to be a third party.

Impact

In the event of a dispute, a buyer acting as an arbitor can award themselves the entire escrow balance, leading to potential financial loss for the seller even if they have every right to a dispute due to doing a perfect job. This vulnerability defeats the purpose of an escrow service and undermines the trustworthiness of the platform.

Tools Used

Manual Audit

Recommended Mitigation

Add an explicit check during contract initialization to prevent the buyer from being the arbiter. Ensure that the buyer and arbitor addresses cannot be the same to prevent potential misuse of dispute resolution. I believe same should be done for the seller address too.

In short this should be implemented buyer != seller != arbiter