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

Fee-on-transfer tokens can not be used and prevent deploying new `Escrow` contracts

bernd

Summary

New Escrow contract instances can not be deployed for tokenContract tokens that charge transfer fees.

Vulnerability Details

Creating a new Escrow contract instance via the EscrowFactory.newEscrow function requires transferring the price amount of tokens (tokenContract) to the new Escrow contract address. However, if the used tokenContract is a fee-on-transfer token (i.e., deducting a transfer fee), the Escrow contract will not receive the full price amount of tokenContract tokens. The balance check in the Escrow contract in line 44 will revert with an error due to receiving slightly less than the price amount of tokenContract tokens.

Please note that $USDT has theoretically the ability to charge transfer fees (see https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7?a=0x522dffc539c264a8f7e0b91102e899b33f4dabc3#code line 131) once fees are enabled. Moreover, $USDC is an upgradeable contract and, theoretically, could also be upgraded to charge transfer fees.

As the protocol is deployed as immutable contracts, it is assumed that it should continue functioning with widely used ERC-20 tokens well into the future. Thus, the consideration of possible ERC-20 token transfer fees is recommended, and medium severity was chosen.

Escrow.sol#L44

32: constructor(
33: uint256 price,
34: IERC20 tokenContract,
35: address buyer,
36: address seller,
37: address arbiter,
38: uint256 arbiterFee
39: ) {
40: if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
41: if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
42: if (seller == address(0)) revert Escrow__SellerZeroAddress();
43: if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
44: @> if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
45: i_price = price;
46: i_tokenContract = tokenContract;
47: i_buyer = buyer;
48: i_seller = seller;
49: i_arbiter = arbiter;
50: i_arbiterFee = arbiterFee;
51: }

Impact

Escrow contract instances can not be deployed with fee-on-transfer tokens as the tokenContract and thus limiting the usability of the Escrow contract.

Tools Used

Manual Review

Recommendations

Consider determining i_price by retrieving the current balance of tokenContract in the Escrow constructor instead of supplying it as the price constructor argument.

Support

FAQs

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