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

Buyer risk associated with unintentionally setting 0 address for arbiter

hash

Summary

The design allows for escrow contracts to be deployed with 0 address as the arbiter and hence doesn't perform 0 address validation for the arbiter. For buyers who unintentionally provides 0 address as the arbiter this is a huge risk.

Vulnerability Details

0 address is a valid arbiter value as of the current design and hence the input arbiter value should not be sanitized for 0 address.

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

For buyers who unintentionally provides 0 address as the arbiter value their funds are now locked. The only option the buyer has is now to either let the funds be locked in the contract or pass it to the seller even in the case the seller doesn't provide a worthy service.

Tools Used

Manual Review

Recommendations

  1. Disallow 0 address for arbiter as such an escrow contract doesn't provide any value to the buyer.

  2. Create a separate state and function intended for the buyers to verify the details before locking in the funds and enabling the escrow contract.

enum State {
Verification,
Created,
Confirmed,
Disputed,
Resolved
}
​
function verifyEscrow() external onlyBuyer inState(State.Verification) {
s_state = State.Created;
}

Support

FAQs

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