Summary
Escrow and Escrow factory can't support ERC20 that have fee-on-transfer functionality.
Vulnerability Details
When escrow created using factory, it doesn't consider if ERC20 take fee on transfer :
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/EscrowFactory.sol#L39-L47
pragma solidity 0.8.18;
import {IEscrowFactory} from "./IEscrowFactory.sol";
import {IEscrow} from "./IEscrow.sol";
import {Escrow} from "./Escrow.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
contract EscrowFactory is IEscrowFactory {
using SafeERC20 for IERC20;
function newEscrow(
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,
bytes32 salt
) external returns (IEscrow) {
address computedAddress = computeEscrowAddress(
type(Escrow).creationCode,
address(this),
uint256(salt),
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
tokenContract.safeTransferFrom(msg.sender, computedAddress, price);
Escrow escrow = new Escrow{salt: salt}(
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
if (address(escrow) != computedAddress) {
revert EscrowFactory__AddressesDiffer();
}
emit EscrowCreated(address(escrow), msg.sender, seller, arbiter);
return escrow;
}
Inside escrow constructor, it check if the balance is lower than the inputted price, it will revert :
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L44
constructor(
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) {
if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}
Impact
This design cause Escrow and Escrow factory completely doesn't support fee on transfer token
Tools Used
Manual review
Recommendations
Consider to check before and after transfer balance when creating escrow, and put the diff of before and after as the price input to Escrow.
