CodeHawks Escrow Contract - Competition Details

Vulnerability Details

Summary

Arbiter, a trusted role, could have a disagreement on the low arbiter fee being compensated to settle a dispute.

Proof of concept

Given the trusted role as an arbiter, the arbiter may not agree with the arbiter fee given to settle a dispute. The arbiter could not do anything about it as the arbiter fee is set during contract creation and could not be changed. Here are the functions which affect the arbiterFee.

From all the functions above, we could not change the arbiter fee once the contract is being deployed.

Workflow:

1. Buyer creates an Escrow via EscrowFactory::newEscrow but the buyer could accidentally/intentionally put in the wrong value for the arbiter fee.

2. Either the buyer or seller initiates a dispute via Escrow::initiateDispute which changes the Escrow's state from Created to Disputed.

3. There are two cases on whether the arbiter resolves the dispute:

   • The arbiter (being a trusted role) confers with both parties offchain. Arbiter then calls Escrow::resolveDispute, reimbursing either side accordingly, emptying the Escrow. This also changes the state of the Escrow contract to Resolved. However, the arbiter is being forced to accept the undesirable low arbiter fee. For instance, it takes the arbiter one week to resolve the dispute between both the buyer and seller, however, due to the incorrect value of arbiter fee, let’s say 100 amount of tokens which is worth $1, the arbiter is being forced to accept this low fee to settle the dispute and was being underpaid due to the error being set by the buyer during contract creation.

   • The arbiter does not settle the dispute (unlikely given the trusted role). The contract is stuck in the Disputed state forever and the funds deposited could not be recovered.

Impact

The arbiter is underpaid after settling the dispute due to an error in contract creation by the buyer.

How to fix

1. Discuss among three parties, buyer, seller, and arbiter before contract creation.

2. Implement a setter function for i_arbiterFee so that the buyer could correct the mistake he/she makes during contract creation.

   uint256 private i_arbiterFee; // remove immutable

   ​

   function setArbiterFee(uint256 arbiterFee) external onlyBuyer {

   i_arbiterFee = arbiterFee;

   }

3. Remove i_arbiterFee variable and add one more parameter, arbiterFee for resolveDispute function. Since the arbiter is a trusted role, the arbiter is reliable to provide the correct arbiterFee while calling Escrow::resolveDispute function. This method also provides flexibility for the arbiter if the arbiter is paid based on hourly rate/daily rate. For instance, the arbiter does not expect that he/she requires more time to handle the dispute and the arbiter could charge more on the arbiter fee.

   function resolveDispute(uint256 buyerAward, uint256 arbiterFee) external onlyArbiter nonReentrant inState(State.Disputed) {

   uint256 tokenBalance = i_tokenContract.balanceOf(address(this));

   uint256 totalFee = buyerAward + arbiterFee; // Reverts on overflow

   if (totalFee > tokenBalance) {

   revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);

   }

   ​

   s_state = State.Resolved;

   emit Resolved(i_buyer, i_seller);

   ​

   if (buyerAward > 0) {

   i_tokenContract.safeTransfer(i_buyer, buyerAward);

   }

   if (arbiterFee > 0) {

   i_tokenContract.safeTransfer(i_arbiter, arbiterFee);

   }

   tokenBalance = i_tokenContract.balanceOf(address(this));

   if (tokenBalance > 0) {

   i_tokenContract.safeTransfer(i_seller, tokenBalance);

   }

   }

Based on the methods above, they could also solve one of the known issues which is Large arbiter fee results in little/no seller payment - In this scenario, the seller can decide to not perform the audit. If this is the case, the only way the buyer can receive any of their funds back is by initiating the dispute process, in which the buyer loses a large portion of their deposited funds to the arbiter. Therefore, the buyer is disincentivized to deploy a new Escrow in such a way.