CodeHawks Escrow Contract - Competition Details

Impact

In the Escrow.constructor the arbiterFee is checked against an upper bound as follows:

    if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);

Hence the arbiterFee should be less than the price amount.

But there is no check on arbiterFee for lower bound value.

Let's consider the following scenario:

1. Buyer deploys the Escrow account by transferring the funds to the Escrow account address.

2. Buyer mistakenly sets arbiterFee to be zero (or any other extremely low value).

3. Escrow.constructor will pass the if conditional check since arbiterFee < price

4. A malicious seller sees this transaction and decides to freeze the funds of the buyer

5. seller does not provide any service to the buyer and immediately calls the Escrow.initiateDispute function and initiates a dispute.

6. Now there is no incentive for the arbiter to resolve the dispute since the arbiterFee == 0.

7. Hence arbiter will neither get involved in the arbitration nor will call the Escrow.resolveDispute to release the funds to either buyer or seller.

8. As a result the buyer funds will be permanently locked in the Escrow contract.

Vulnerability Details

https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L40-L44

Tools Used

Manual Review and VSCode

Recommendations

Hence it is recommended to implement a lower bound for the arbiterFee as well. This can be a percentage, such as 2.5% of the price and should be save in an immutable variable i_minimumArbiterFee. Hence the arbiterFee should be checked, that it is greater than the lower bound (i_minimumArbiterFee) inside the Escrow.constructor and if not the deployment should revert. Below require statement should be added in the Escrow.constructor.

require(arbiterFee > i_minimumArbiterFee, "arbiter is not incentivized to perform action");