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

All funds can get stuck if a dispute is initiated and either the Arbiter or the Seller gets blacklisted in the underlying i_tokenContract

0xstalin

Summary

  • Funds can get stuck in the Escrow contract if a dispute is initiated and either the Arbiter or the Seller gets blacklisted in the contract of the price token

Vulnerability Details

  • When a dispute is initiated, currently, the only path to resolve it is when the arbiters call the Escrow:resolveDispute().

    • The arbiter has control over how many tokens will be refunded to the buyer (if any).

    • The resolveDispute() will attempt to transfer the refunds to the buyer (if any), the fees to the arbiter, and the remaining tokens in the contract's balance will be sent to the seller.

  • The problem is that the resolveDispue() is pushing the tokens from the contract to the accounts, if the prize token (i_tokenContract) is a token like USDC or USDT that has blacklisted functionalities it opens the risk of blocking intentionally or by accident all the funds in the Escrow contract when a dispute is initiated.

    • Additionally, there is a check to prevent the totalFee (which is the sum of the refunded amount to the buyer + the arbiter's fees) is not greater than the tokenBalance, which means, that after refunding the buyer and paying the arbiter's fees will be left over at least 1 wei in the contract, and this 1 wei will be attempted to be transferred to the seller!

Suppose the below scenarios:

  1. The dispute was opened by the buyer, and as per the Arbiter's judgment the buyer is right and it should get refunded X amount of tokens (it could be a 100% of the contract's token balance).

  • After a long back-and-forth discussion, the seller just doesn't accept the arbiter's judgment.

  • The arbiter has decided to resolve the dispute and refund the X amount to the buyer.

  • The seller, instead of accepting the arbiter's judgment, blacklists his address in the underlying prize token.

  • When the arbiter attempts to resolve the dispute, the tx will be reverted because the seller's address is blacklisted in the prize token.

  • As a result, even if the arbiter has taken a decision to resolve the dispute, the dispute won't be able to be resolved and the funds will be stuck in the Escrow contract.

  1. The arbiter's address gets blacklisted in the prize token contract (i_tokenContract) and a dispute is opened.

  • If the arbiter's address is blacklisted & there are any arbiter fees (i_arbiterFee), the dispute won't be able to be resolved and the funds will be stuck in the Escrow contract

There are a number of widely used ERC20 tokens that have blacklists (e.g. USDC, USDT). Reference

Tools Used

Manual Audit

Recommendations

  • Implement a pull pattern instead of a push pattern to distribute the tokens when disputes are resolved

    • Allow each user to claim individually their respective portion of the prize

      • In this way, if one of the user's addresses is blacklisted it won't affect the process to resolve the dispute, and the rest of the users won't be affected to claim their respective tokens

  1. Create a new mapping to keep track of the amount of funds that each entity will be allowed to claim

mapping(address => uint256) claimAmounts;

  1. In the resolveDispute(), instead of sendings the tokens to the accounts, update the new claimAmounts mapping, so the users can later claim their respective tokens

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) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}
​
s_state = State.Resolved;
emit Resolved(i_buyer, i_seller);
​
if (buyerAward > 0) {
- i_tokenContract.safeTransfer(i_buyer, buyerAward);
+ claimAmounts[i_buyer] = buyerAward;
}
if (i_arbiterFee > 0) {
- i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
+ claimAmounts[i_arbiter] = i_arbiterFee;
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
- i_tokenContract.safeTransfer(i_seller, tokenBalance);
+ claimAmounts[i_seller] = tokenBalance;
}
}

  1. Create a function to allow the users to claim their funds

function claimFunds() external nonReentrant {
if(msg.sender == i_buyer) {
uint256 buyerAward = claimAmounts[i_buyer];
claimAmounts[i_buyer] = 0;
if (buyerAward != 0) i_tokenContract.safeTransfer(i_buyer, buyerAward);
} else if(msg.sender == i_arbiter) {
uint256 arbiterAward = claimAmounts[i_arbiter];
claimAmounts[i_arbiter] = 0;
if (arbiterAward != 0) i_tokenContract.safeTransfer(i_arbiter, arbiterAward);
} else if(msg.sender == i_seller) {
uint256 sellerAward = claimAmounts[i_seller];
claimAmounts[i_seller] = 0;
if (sellerAward != 0) i_tokenContract.safeTransfer(i_seller, sellerAward);
} else {
revert();
}
};

  • Optionally, you could allow the users to specify the address where they want to receive their funds

  1. Create a function to allow the users to claim their funds

function claimFunds(address _receiver) external nonReentrant {
if(msg.sender == i_buyer) {
uint256 buyerAward = claimAmounts[i_buyer];
claimAmounts[i_buyer] = 0;
if (buyerAward != 0) i_tokenContract.safeTransfer(_receiver, buyerAward);
} else if(msg.sender == i_arbiter) {
uint256 arbiterAward = claimAmounts[i_arbiter];
claimAmounts[i_arbiter] = 0;
if (arbiterAward != 0) i_tokenContract.safeTransfer(_receiver, arbiterAward);
} else if(msg.sender == i_seller) {
uint256 sellerAward = claimAmounts[i_seller];
claimAmounts[i_seller] = 0;
if (sellerAward != 0) i_tokenContract.safeTransfer(_receiver, sellerAward);
} else {
revert();
}
};

Support

FAQs

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