CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

In some cases buyers won't be able to get their funds back without putting their trust on the sellers to voluntarily return them back.

Yuki

Summary

In some cases buyers won't be able to get their funds back without putting their trust on the sellers to voluntarily return them back.

Vulnerability Details

Currently the contract allows deploying an escrow with address(0) as arbiter.

Not using an arbiter could be a result by either of the parties having trust issues towards a third person to allocate the right amount of tokens when dispute is resolved. Thats why there is an option to deploy an escrow contract with address(0) as arbiter, this indicates that arbiter won't be used and initiateDispute can't be made.

However there is a big downside of not using arbiter, which the current system doesn't predict:

  1. Buyer and seller make an agreement for a private audit and deploys a new escrow contract with address(0), as for some reason either of the parties have a trust issue towards third party. The two parties schedule a week on which the private audit will be done.

  2. The scheduled time comes and for some reason the seller doesn't want to do the private audit anymore. It could be that the auditor simply has something else to do or just a protocol with better offer showed up.

  3. As there is no arbiter, a dispute can't be initiated to return back the tokens to the buyer.

  4. The only available way to send the funds outside of the escrow, would be to call confirmReceipt and send the amount of tokens the contract has to the seller.

  5. In the end there is no guarantee that the seller will voluntarily return the tokens back to the buyer.

function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();
s_state = State.Disputed;
emit Disputed(msg.sender);
}

Impact

In a case when an arbiter is not used in the escrow and the private audit is not performed. The only way for the buyer to receive his funds back from the escrow would be to put trust on the seller to voluntarily return them back.

Tools Used

Manual review.

Recommend Mitigation

The only recommended fix l could've think of is to not allow an escrow to be deployed without an arbiter.

constructor(
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) {
if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
+ if (arbiter == address(0)) revert Escrow__ArbiterZeroAddress();
if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}

Support

FAQs

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