CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: medium

Seller might get overpaid while settling the dispute

neocrao

Summary

Seller might get overpaid while settling the dispute

Vulnerability Details

The resolveDispute() method outlines a couple of issues that may still get the seller paid the i_price amount (or even more) despite a award/refund to the buyer and the arbiter fee.

There are two potential causes for it:

  • Usage of uint256 tokenBalance = i_tokenContract.balanceOf(address(this));

  • Transferring i_tokenContract.balanceOf(address(this)) to the seller

Suppose:

  • The i_price was set to 100 tokens.

  • The i_arbiterFee is set to 10 tokens.

Now, if during the deployment of the the Escrow contract, lets say it was funded multiple times - either by mistake, or some issues that could have risen in the Factory. Lets say the contract address balance started with 20 tokens.
Upon creation of the contract, the final balance of the contract becomes 20 tokens + 100 tokens = 120 tokens (i_price + initial balance)

Now, lets say that the arbiter calls resolveDispute with 5 tokens as the buyer award.

This passes all the checks, and the following transfers happen:

  • The buyer gets 5 tokens

  • The arbiter gets 10 tokens

  • The seller gets the remaining contract balance, that is 120 - 5 - 10 = 105 tokens, which is clearly more than i_price

Impact

Even if a arbiter is involved, there is a chance that the seller gets paid in full, or even more, despite there being an arbiter fee, and/or a buyer award/refund.

Severity Justification

Marking this as medium as both the following medium criteria satisfy:

  • Funds are indirectly at risk

  • Disruption of protocol functionality or availability

Source: https://docs.codehawks.com/rewards-and-judging

Tools Used

Manual analysis

Recommendations

Update the resolveDispute() method as follows:

function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
- uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
- if (totalFee > tokenBalance) {
+ if (totalFee > i_price) {
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 (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);
+ i_tokenContract.safeTransfer(i_seller, i_price - (buyerAward + i_arbiterFee));
}
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, i_tokenContract.balanceOf(address(this)));
}
}

Support

FAQs

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