Summary

The issue arises when the buyer maliciously calls the initiateDispute() function without facing any consequences, leading to a reduction in the funds received by the seller. This vulnerability is not beneficial for the buyer but is still possible to happen intentionally or unintentionally.

The Proof of Concept demonstrates a scenario where the seller audits a project perfectly, and the buyer receives the results. However, the buyer maliciously calls the initiateDispute() function, and after dispute resolution, the seller loses part of the funds to the arbiterFee.

The recommended mitigation involves modifying the contract to prevent malicious calls to the initiateDispute() function by the buyer. A mechanism is proposed where the buyer sends i_price + i_arbiterFee to the contract when creating the Escrow contract. If the arbiter determines that the buyer's call was malicious, the arbiterFee will be taken from the buyer. Otherwise, if the call was legitimate, the arbiter will decide how much of the arbiterFee should be paid by each party. The contract functions confirmReceipt() and resolveDispute() are updated to implement this mechanism.

In summary, the proposed modification aims to prevent malicious behavior from the buyer during disputes and ensure fair distribution of funds in the Escrow contract when disputes occur.

Vulnerability Details

Both buyer and seller can call the initiateDispute() function. After calling this function, the arbiter involves solves the dispute and decides to pay each party a specific share of the funds. When no disputes are initiated, all of the funds will be transferred to the seller. However, in case of the occurrence of any disputes, some of the funds will be transferred to the arbiter as the arbiterFee. The remainings will be split based on the decision of the arbiter.

The issue is when the buyer is malicious. The buyer always has the ability to call the initiateDispute() function without any consequences. The seller does not have the incentive to call this function, since the funds that the seller is going to receive will be decreased. However, when the buyer calls the initiateDispute() function, the buyer does not lose any funds, but some of the seller's funds will be transferred to the arbiter.

It may not be beneficial for a buyer to do this; however, it is not impossible to happen. The buyer can call it intentionally or unintentionally.

Impact

Consider the case that Alice is the buyer and offers $10,000 for an audit. Bob accepts the opportunity and the Escrow contract will be created with $10,000 funds locked in it. Also, Charlie is the arbiter, and in case of dispute, receives $1,000 to solve it.

Consider Bob audits the projects perfectly, and sends the results to Alice. However, Alice is a malicious party and decides to call the intiateDispute() function. Afterward, the dispute will be created, and Charlie judges it and decides that all of the funds must be transferred to Bob, who audited the project perfectly.

However, Bob will receive $9,000 and loses $1,000, since $1,000 is transferred to Charlie's account as the arbiterFee.

Tools Used

Manual Review

Recommendations

Consider implementing a mechanism that can prevent the buyer to call the initiateDisputre() function maliciously. I suggest the modification below:

1. Since seller loses funds if calls the initiateDispute() function, this party does not call this function maliciously.

2. When the buyer creates the Escrow contract, this party must send i_price + i_arbiterFee to the contract. Change EscrowFactory.sol#L39 to the line below:

   tokenContract.safeTransferFrom(msg.sender, computedAddress, price + arbiterFee);

3. Then, two cases can occur:

   1. buyer calls the initiateDispute() function maliciously. In this case, if arbiter decides that the call from the buyer is malicious, takes all of the arbiterFee from the buyer that was sent when the contract was created.

   2. buyer calls the initiateDispute() function correctly. In this case, arbiter decides how much each party must pay the arbiterFee. Usually, 50% must be paid by the buyer and 50% by the seller.

   Change confirmReceipt() function to the code below:

   function confirmReceipt() external onlyBuyer inState(State.Created) {

   s_state = State.Confirmed;

   emit Confirmed(i_seller);

   ​

   i_tokenContract.safeTransfer(i_buyer, i_arbiterFee);

   i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)) - i_arbiterFee);

   }

   Also, change resolveDispute() function to the code below:

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

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

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

   if (totalFee > tokenBalance) {

   revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);

   }

   ​

   s_state = State.Resolved;

   emit Resolved(i_buyer, i_seller);

   // If buyer is malicious, this if condition will not be executed and the buyer only receives the `buyerAward`

   // If buyer is not malicious, buyer will pay half of the `arbiterFee` and the other half is being paid by `seller`

   if (!isBuyerMalicious) buyerAward += i_arbiterFee / 2;

   if (buyerAward > 0) {

   i_tokenContract.safeTransfer(i_buyer, buyerAward);

   }

   if (i_arbiterFee > 0) {

   i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);

   }

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

   if (tokenBalance > 0) {

   i_tokenContract.safeTransfer(i_seller, tokenBalance);

   }

   }