Preventing Fund Lockup: Ensuring a Valid Arbiter in the Escrow Contract Constructor

Severity

High Risk

Relevant GitHub Links

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

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

Summary

The constructor for the escrow contract does not validate the arbiter's address, allowing for the creation of an escrow contract without a valid arbiter. This could prevent disputes from being initiated, locking funds indefinitely.

Vulnerability Details

The constructor for the escrow contract does not check if the arbiter parameter is a non-zero address:

constructor(
    uint256 price,
    IERC20 tokenContract,
    address buyer,
    address seller,
    address arbiter,
    uint256 arbiterFee
) {
    ...
    i_arbiter = arbiter;
    ...
}

This allows for the creation of an escrow contract without an arbiter. However, the initiateDispute function requires a valid arbiter:

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

This could lead to a situation where funds are locked in the contract indefinitely, as disputes cannot be initiated without a valid arbiter.

Impact

If an escrow contract is created without a valid arbiter, funds could be locked indefinitely. This would occur if a dispute arises and the initiateDispute function cannot be called due to the lack of a valid arbiter.

Tools Used

This potential vulnerability was found using manual code review methods.

Recommendations

I recommend adding a check in the constructor to ensure that the arbiter parameter is a non-zero address. This will prevent the creation of an escrow contract without a valid arbiter, and will ensure that disputes can always be initiated when necessary. Here's how you can modify your constructor:

constructor(
    uint256 price,
    IERC20 tokenContract,
    address buyer,
    address seller,
    address arbiter,
    uint256 arbiterFee
) {
    ...
    if (arbiter == address(0)) revert Escrow__ArbiterZeroAddress();
    ...
}

Cyfrin / 2023-07-escrow