40,000 USDC
View results
Submission Details
Severity: high

`arbiterFee` should not be taken blindly from the initial allocation amount for seller, instead it should be taken from the party who initiate the dispute process

Summary

arbiterFee should not be taken blindly from the initial allocation amount for seller, instead it should be taken from the party who initiate the dispute process

Vulnerability Details

Currently, the arbiterFee is forcibly deducted from the tokenBalance. This means, when initially seller promised to be paid X amount, if there is a dispute, the arbiterFee will be taken from this X amount, thus decreasing seller potential earning.

The dispute process should not reduce the initial agreed-upon amount between the buyer and seller (just in case, the Arbiter later decide take on seller side, as the seller fairly deliver the service, thus should get full amount). Instead, the arbiter's fee should be handled separately and not impact the original agreement between the parties. This ensures that the escrow remains fair and transparent, without altering the core terms of the transaction during the dispute resolution.

Since dispute can be called by both buyer or seller, it's not fair if the fee is deducted only from seller allocation. Therefore, in a dispute process, the fee should be deducted from the one who initiate dispute.

File: Escrow.sol
109: function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
110: uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
111: uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
112: if (totalFee > tokenBalance) {
113: revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
114: }
...
119: if (buyerAward > 0) {
120: i_tokenContract.safeTransfer(i_buyer, buyerAward);
121: }
122: if (i_arbiterFee > 0) {
123: i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
124: }
125: tokenBalance = i_tokenContract.balanceOf(address(this));
126: if (tokenBalance > 0) {
127: i_tokenContract.safeTransfer(i_seller, tokenBalance);
128: }
129: }

Since there is no check who initiate the dispute, and the tokenBalance is actually the agreed amount for seller when there is no dispute, when buyer initiate dispute there is no code to 'increase' balance, thus resolveDispute implicitly shows that arbiterFee is just taken from tokenBalance.

An edge case scenario:

  1. Alice as buyer agree to hire Bob for a service of $1000, using third party Escrow $100

  2. Bob deliver his service on time, but Alice didn't get it (e.g, delivering email but got in to spam folder)

  3. Alice initiate dispute process (thinking that Bob failed to deliver the service on time)

  4. But Alice by herself, noticed her own fault, and then agree to release the fund (told Arbiter to give Bob his full earning)

  5. Arbiter call resolveDispute and try to give full amount of Bob earnings

  6. Since it was disputed, Bob will get $900 because a cut $100 from Escrow, while in fact, it was Alice fault, and Bob deliver it on time.

  7. Ideally, Bob should get his $1000, and Alice is the one who paid $100 to Escrow, due to her own fault.

Impact

Seller will get less amount due to cut from Escrow, even though the fault is on Buyer which is not fair.

Tools Used

Manual

Recommendations

The initial price a buyer need to send to contract when Arbiter selected is, Buyer Amount + Arbiter Fee (just in case seller will get the full amount).

When the dispute in place (assuming seller will get full amount):

  1. If dispute initiated by Buyer: Arbiter is paid first, then send amount to seller in full, if there are some amount left in the contract, return to buyer.

  2. If dispute initiated by Seller: Arbiter is paid first, send amount to seller minus arbiterFee, return to buyer if there are some excess.

Support

FAQs

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