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

Permanent locking of funds in Escrows

Summary

Even if the option for disabling the arbitration mechanism was a design choice, without that mechanism enabled, the funds locked in an Escrow contract are at risk of permanently locking away (please refer to the Impact section for further explanation).

Vulnerability Details

The Escrow contract was designed to have the arbitration mechanism optional. That means a buyer can create an Escrow contract by passing an arbiter address as address(0) to opt out of the arbitration option. In other words, neither a buyer nor a seller can raise a dispute by executing the initiateDispute() since the transaction will be reverted in line 103.

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

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L103

Impact

Even if the option for disabling the arbitration mechanism was a design choice, without that mechanism enabled, the funds locked in an Escrow contract are at risk of permanently locking away.

Consider the following scenarios.

  1. If a rogue seller refuses to deliver a service, the buyer's funds will be locked in the contract forever. The buyer will have no way to retrieve their funds.

  2. If a rogue buyer receives a service but refuses to execute the confirmReceipt(), the locked funds will never be transferred to a seller. The seller will have no way of disputing, moreover.

Tools Used

Manual Review

Recommendations

I recommend explicitly applying the arbitration mechanism for every Escrow contract.

Support

FAQs

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